cmd/go/internal/modload: break more cycles in readModGraph

Before CL 471595, modload.readModGraph in module with graph pruning
enabled only ever chased down transitive dependencies of unpruned
roots, so pruned dependencies couldn't cause cycles and we didn't
need to dedup them in the loading queue.

However, in 'go get' we are now passing in a set of upgraded modules
to unprune, and those upgraded modules can potentially contain cycles,
leading to an infinite loop during loading.

We have two options for a fix: we could either drop the 'unprune'
check in the enqueue operation (and instead expand the 'unprune'
requirements in a separate pass, as we do in workspace mode), or we
could check for cycles for all modules (not just the ones that are
naturally unpruned). The latter option makes it clearer that this
process must terminate, so we choose that.

(It may be possible to clean up and simplify the workspace-mode case
now that we are passing in the 'unprune' map, but for now we're
looking for a minimal fix for the Go 1.21 release.)

Fixes #60490.

Change-Id: I701f5d43a35e357f6c0c0c9d10b7aa088f917311
Reviewed-on: https://go-review.googlesource.com/c/go/+/499195
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>

diff --git a/src/cmd/go/internal/modload/buildlist.go b/src/cmd/go/internal/modload/buildlist.go
index 7cebb9f265ae3179fb5f9112819cf95931daef0c..76500ab33f7282dfed5c0dccfa59f74fbdbdc35d 100644 (file)
--- a/src/cmd/go/internal/modload/buildlist.go
+++ b/src/cmd/go/internal/modload/buildlist.go
@@ -315,9 +315,13 @@ func readModGraph(ctx context.Context, pruning modPruning, roots []module.Versio
                mg.g.Require(MainModules.mustGetSingleMainModule(), roots)
        }
 
+       type dedupKey struct {
+               m       module.Version
+               pruning modPruning
+       }
        var (
-               loadQueue       = par.NewQueue(runtime.GOMAXPROCS(0))
-               loadingUnpruned sync.Map // module.Version → nil; the set of modules that have been or are being loaded via roots that do not support pruning
+               loadQueue = par.NewQueue(runtime.GOMAXPROCS(0))
+               loading   sync.Map // dedupKey → nil; the set of modules that have been or are being loaded
        )
 
        // loadOne synchronously loads the explicit requirements for module m.
@@ -345,13 +349,11 @@ func readModGraph(ctx context.Context, pruning modPruning, roots []module.Versio
                        return
                }
 
-               if pruning == unpruned {
-                       if _, dup := loadingUnpruned.LoadOrStore(m, nil); dup {
-                               // m has already been enqueued for loading. Since unpruned loading may
-                               // follow cycles in the requirement graph, we need to return early
-                               // to avoid making the load queue infinitely long.
-                               return
-                       }
+               if _, dup := loading.LoadOrStore(dedupKey{m, pruning}, nil); dup {
+                       // m has already been enqueued for loading. Since unpruned loading may
+                       // follow cycles in the requirement graph, we need to return early
+                       // to avoid making the load queue infinitely long.
+                       return
                }
 
                loadQueue.Add(func() {

diff --git a/src/cmd/go/testdata/script/mod_get_issue60490.txt b/src/cmd/go/testdata/script/mod_get_issue60490.txt
new file mode 100644 (file)
index 0000000..e0ac26a
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_get_issue60490.txt
@@ -0,0 +1,48 @@
+# Regression test for https://go.dev/issue/60490: 'go get' should not cause an
+# infinite loop for cycles introduced in the pruned module graph.
+
+go get example.net/c@v0.1.0
+
+-- go.mod --
+module example
+
+go 1.19
+
+require (
+       example.net/a v0.1.0
+       example.net/b v0.1.0
+)
+
+replace (
+       example.net/a v0.1.0 => ./a1
+       example.net/a v0.2.0 => ./a2
+       example.net/b v0.1.0 => ./b1
+       example.net/b v0.2.0 => ./b2
+       example.net/c v0.1.0 => ./c1
+)
+-- a1/go.mod --
+module example.net/a
+
+go 1.19
+-- a2/go.mod --
+module example.net/a
+
+go 1.19
+
+require example.net/b v0.2.0
+-- b1/go.mod --
+module example.net/b
+
+go 1.19
+-- b2/go.mod --
+module example.net/b
+
+go 1.19
+
+require example.net/a v0.2.0
+-- c1/go.mod --
+module example.net/c
+
+go 1.19
+
+require example.net/a v0.2.0