internal/gcimporter: another workaround for race to interface type set
When importing, it is also possible to produce an incomplete interface
instance, which can be racy (as reproduced in the included test).
Partially avoid this by completing interface underlyings of instances.
For golang/go#61561
Change-Id: I8ca2bdc2d03fa1a46179bbd8596d7ef9baf51600
Reviewed-on: https://go-review.googlesource.com/c/tools/+/512955
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/internal/gcimporter/gcimporter_test.go b/internal/gcimporter/gcimporter_test.go
index 2473a0e..1407e90 100644
--- a/internal/gcimporter/gcimporter_test.go
+++ b/internal/gcimporter/gcimporter_test.go
@@ -25,6 +25,7 @@
"runtime"
"sort"
"strings"
+ "sync"
"testing"
"time"
@@ -769,6 +770,72 @@
_ = importPkg(t, "./testdata/aa", tmpdir)
}
+func TestIssue61561(t *testing.T) {
+ testenv.NeedsGo1Point(t, 18) // requires generics
+
+ const src = `package p
+
+type I[P any] interface {
+ m(P)
+ n() P
+}
+
+type J = I[int]
+
+type StillBad[P any] *interface{b(P)}
+
+type K = StillBad[string]
+`
+ fset := token.NewFileSet()
+ f, err := goparser.ParseFile(fset, "p.go", src, 0)
+ if f == nil {
+ // Some test cases may have parse errors, but we must always have a
+ // file.
+ t.Fatalf("ParseFile returned nil file. Err: %v", err)
+ }
+
+ config := &types.Config{}
+ pkg1, err := config.Check("p", fset, []*ast.File{f}, nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ // Export it. (Shallowness isn't important here.)
+ data, err := IExportShallow(fset, pkg1, nil)
+ if err != nil {
+ t.Fatalf("export: %v", err) // any failure to export is a bug
+ }
+
+ // Re-import it.
+ imports := make(map[string]*types.Package)
+ pkg2, err := IImportShallow(fset, GetPackagesFromMap(imports), data, "p", nil)
+ if err != nil {
+ t.Fatalf("import: %v", err) // any failure of IExport+IImport is a bug.
+ }
+
+ insts := []types.Type{
+ pkg2.Scope().Lookup("J").Type(),
+ // This test is still racy, because the incomplete interface is contained
+ // within a nested type expression.
+ //
+ // Uncomment this once golang/go#61561 is fixed.
+ // pkg2.Scope().Lookup("K").Type().Underlying().(*types.Pointer).Elem(),
+ }
+
+ // Use the interface instances concurrently.
+ for _, inst := range insts {
+ var wg sync.WaitGroup
+ for i := 0; i < 2; i++ {
+ wg.Add(1)
+ go func() {
+ defer wg.Done()
+ _ = types.NewMethodSet(inst)
+ }()
+ }
+ wg.Wait()
+ }
+}
+
func TestIssue57015(t *testing.T) {
testenv.NeedsGo1Point(t, 18) // requires generics
diff --git a/internal/gcimporter/iimport.go b/internal/gcimporter/iimport.go
index 52faa77..8e64cf6 100644
--- a/internal/gcimporter/iimport.go
+++ b/internal/gcimporter/iimport.go
@@ -328,6 +328,13 @@
typ.Complete()
}
+ // Workaround for golang/go#61561. See the doc for instanceList for details.
+ for _, typ := range p.instanceList {
+ if iface, _ := typ.Underlying().(*types.Interface); iface != nil {
+ iface.Complete()
+ }
+ }
+
return pkgs, nil
}
@@ -358,6 +365,12 @@
fake fakeFileSet
interfaceList []*types.Interface
+ // Workaround for the go/types bug golang/go#61561: instances produced during
+ // instantiation may contain incomplete interfaces. Here we only complete the
+ // underlying type of the instance, which is the most common case but doesn't
+ // handle parameterized interface literals defined deeper in the type.
+ instanceList []types.Type // instances for later completion (see golang/go#61561)
+
// Arguments for calls to SetConstraint that are deferred due to recursive types
later []setConstraintArgs
@@ -954,6 +967,9 @@
// we must always use the methods of the base (orig) type.
// TODO provide a non-nil *Environment
t, _ := typeparams.Instantiate(nil, baseType, targs, false)
+
+ // Workaround for golang/go#61561. See the doc for instanceList for details.
+ r.p.instanceList = append(r.p.instanceList, t)
return t
case unionType:
