]> Cypherpunks repositories - gostls13.git/commitdiff
go/types, types2: instantiated interfaces must be concurrency safe
authorRob Findley <rfindley@google.com>
Tue, 25 Jul 2023 15:57:37 +0000 (11:57 -0400)
committerGopher Robot <gobot@golang.org>
Tue, 1 Aug 2023 17:16:43 +0000 (17:16 +0000)
It is the responsibility of go/types to complete any interface it
creates, except for those created by the user using NewInterface.
However, this was not being done for interfaces created during
instantiation.

Fix this by (rather carefully) ensuring that all newly created
interfaces are eventually completed.

Fixes golang/go#61561

Change-Id: I3926e7c9cf80714838d2c1b5f36a2d3221c60c41
Reviewed-on: https://go-review.googlesource.com/c/go/+/513015
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
src/cmd/compile/internal/types2/api_test.go
src/cmd/compile/internal/types2/interface.go
src/cmd/compile/internal/types2/named.go
src/cmd/compile/internal/types2/subst.go
src/go/types/api_test.go
src/go/types/interface.go
src/go/types/named.go
src/go/types/subst.go

index bf807c35be1b443133999e5dbf0c9add874d7b76..c7a24fc3e5aaaaed8bbb5fa5c6c3a35b6948f33d 100644 (file)
@@ -13,6 +13,7 @@ import (
        "regexp"
        "sort"
        "strings"
+       "sync"
        "testing"
 
        . "cmd/compile/internal/types2"
@@ -2295,6 +2296,60 @@ func TestInstantiate(t *testing.T) {
        }
 }
 
+func TestInstantiateConcurrent(t *testing.T) {
+       const src = `package p
+
+type I[P any] interface {
+       m(P)
+       n() P
+}
+
+type J = I[int]
+
+type Nested[P any] *interface{b(P)}
+
+type K = Nested[string]
+`
+       pkg := mustTypecheck(src, nil, nil)
+
+       insts := []*Interface{
+               pkg.Scope().Lookup("J").Type().Underlying().(*Interface),
+               pkg.Scope().Lookup("K").Type().Underlying().(*Pointer).Elem().(*Interface),
+       }
+
+       // Use the interface instances concurrently.
+       for _, inst := range insts {
+               var (
+                       counts  [2]int      // method counts
+                       methods [2][]string // method strings
+               )
+               var wg sync.WaitGroup
+               for i := 0; i < 2; i++ {
+                       i := i
+                       wg.Add(1)
+                       go func() {
+                               defer wg.Done()
+
+                               counts[i] = inst.NumMethods()
+                               for mi := 0; mi < counts[i]; mi++ {
+                                       methods[i] = append(methods[i], inst.Method(mi).String())
+                               }
+                       }()
+               }
+               wg.Wait()
+
+               if counts[0] != counts[1] {
+                       t.Errorf("mismatching method counts for %s: %d vs %d", inst, counts[0], counts[1])
+                       continue
+               }
+               for i := 0; i < counts[0]; i++ {
+                       if m0, m1 := methods[0][i], methods[1][i]; m0 != m1 {
+                               t.Errorf("mismatching methods for %s: %s vs %s", inst, m0, m1)
+                       }
+               }
+       }
+}
+
 func TestInstantiateErrors(t *testing.T) {
        tests := []struct {
                src    string // by convention, T must be the type being instantiated
index 872a3217c22d1bde37907ae258030d8f13f12554..6623ff557533b6e5bde58adb3cbb773a55b8461b 100644 (file)
@@ -112,6 +112,7 @@ func (t *Interface) String() string   { return TypeString(t, nil) }
 // Implementation
 
 func (t *Interface) cleanup() {
+       t.typeSet() // any interface that escapes type checking must be safe for concurrent use
        t.check = nil
        t.embedPos = nil
 }
index 5408c7e77f2201eee3fc0850db36aae55ecee795..7c9a46f2315afb342b2945d3f996f6fa537c2829 100644 (file)
@@ -633,11 +633,18 @@ func (n *Named) expandUnderlying() Type {
                                old := iface
                                iface = check.newInterface()
                                iface.embeddeds = old.embeddeds
+                               assert(old.complete) // otherwise we are copying incomplete data
                                iface.complete = old.complete
                                iface.implicit = old.implicit // should be false but be conservative
                                underlying = iface
                        }
                        iface.methods = methods
+                       iface.tset = nil // recompute type set with new methods
+
+                       // If check != nil, check.newInterface will have saved the interface for later completion.
+                       if check == nil { // golang/go#61561: all newly created interfaces must be fully evaluated
+                               iface.typeSet()
+                       }
                }
        }
 
index 74d6294dff03f56e4d8f2d43159367ea7bb1dd16..aefa53603ff91e7005a6160bf7a133225dcfea10 100644 (file)
@@ -170,6 +170,7 @@ func (subst *subster) typ(typ Type) Type {
                        iface := subst.check.newInterface()
                        iface.embeddeds = embeddeds
                        iface.implicit = t.implicit
+                       assert(t.complete) // otherwise we are copying incomplete data
                        iface.complete = t.complete
                        // If we've changed the interface type, we may need to replace its
                        // receiver if the receiver type is the original interface. Receivers of
@@ -185,6 +186,11 @@ func (subst *subster) typ(typ Type) Type {
                        // need to create new interface methods to hold the instantiated
                        // receiver. This is handled by Named.expandUnderlying.
                        iface.methods, _ = replaceRecvType(methods, t, iface)
+
+                       // If check != nil, check.newInterface will have saved the interface for later completion.
+                       if subst.check == nil { // golang/go#61561: all newly created interfaces must be completed
+                               iface.typeSet()
+                       }
                        return iface
                }
 
index 363e6d48e942f2c9442b19abc0f554b8c0ea10db..cb1263863f19ce38ce81b7843ee24de11a781854 100644 (file)
@@ -16,6 +16,7 @@ import (
        "regexp"
        "sort"
        "strings"
+       "sync"
        "testing"
 
        . "go/types"
@@ -2300,6 +2301,60 @@ func TestInstantiate(t *testing.T) {
        }
 }
 
+func TestInstantiateConcurrent(t *testing.T) {
+       const src = `package p
+
+type I[P any] interface {
+       m(P)
+       n() P
+}
+
+type J = I[int]
+
+type Nested[P any] *interface{b(P)}
+
+type K = Nested[string]
+`
+       pkg := mustTypecheck(src, nil, nil)
+
+       insts := []*Interface{
+               pkg.Scope().Lookup("J").Type().Underlying().(*Interface),
+               pkg.Scope().Lookup("K").Type().Underlying().(*Pointer).Elem().(*Interface),
+       }
+
+       // Use the interface instances concurrently.
+       for _, inst := range insts {
+               var (
+                       counts  [2]int      // method counts
+                       methods [2][]string // method strings
+               )
+               var wg sync.WaitGroup
+               for i := 0; i < 2; i++ {
+                       i := i
+                       wg.Add(1)
+                       go func() {
+                               defer wg.Done()
+
+                               counts[i] = inst.NumMethods()
+                               for mi := 0; mi < counts[i]; mi++ {
+                                       methods[i] = append(methods[i], inst.Method(mi).String())
+                               }
+                       }()
+               }
+               wg.Wait()
+
+               if counts[0] != counts[1] {
+                       t.Errorf("mismatching method counts for %s: %d vs %d", inst, counts[0], counts[1])
+                       continue
+               }
+               for i := 0; i < counts[0]; i++ {
+                       if m0, m1 := methods[0][i], methods[1][i]; m0 != m1 {
+                               t.Errorf("mismatching methods for %s: %s vs %s", inst, m0, m1)
+                       }
+               }
+       }
+}
+
 func TestInstantiateErrors(t *testing.T) {
        tests := []struct {
                src    string // by convention, T must be the type being instantiated
index f2bb15e84bad06f455222f685f53d840e3b60fa4..5fe9b57c3f0d5767e9e002b796d964b8cbb012f4 100644 (file)
@@ -151,6 +151,7 @@ func (t *Interface) String() string   { return TypeString(t, nil) }
 // Implementation
 
 func (t *Interface) cleanup() {
+       t.typeSet() // any interface that escapes type checking must be safe for concurrent use
        t.check = nil
        t.embedPos = nil
 }
index 413eaada271832f03d6ee38f2bc86c14ad91d88b..fae73412342af02569b4b94a1266259ff00d8d60 100644 (file)
@@ -635,11 +635,18 @@ func (n *Named) expandUnderlying() Type {
                                old := iface
                                iface = check.newInterface()
                                iface.embeddeds = old.embeddeds
+                               assert(old.complete) // otherwise we are copying incomplete data
                                iface.complete = old.complete
                                iface.implicit = old.implicit // should be false but be conservative
                                underlying = iface
                        }
                        iface.methods = methods
+                       iface.tset = nil // recompute type set with new methods
+
+                       // If check != nil, check.newInterface will have saved the interface for later completion.
+                       if check == nil { // golang/go#61561: all newly created interfaces must be fully evaluated
+                               iface.typeSet()
+                       }
                }
        }
 
index 30c48e1badc4bd628010a83e006cbf2892ddae3f..13d3dcbf1eac6e77a808221312aae44c1eeb838b 100644 (file)
@@ -172,6 +172,7 @@ func (subst *subster) typ(typ Type) Type {
                        iface := subst.check.newInterface()
                        iface.embeddeds = embeddeds
                        iface.implicit = t.implicit
+                       assert(t.complete) // otherwise we are copying incomplete data
                        iface.complete = t.complete
                        // If we've changed the interface type, we may need to replace its
                        // receiver if the receiver type is the original interface. Receivers of
@@ -187,6 +188,11 @@ func (subst *subster) typ(typ Type) Type {
                        // need to create new interface methods to hold the instantiated
                        // receiver. This is handled by Named.expandUnderlying.
                        iface.methods, _ = replaceRecvType(methods, t, iface)
+
+                       // If check != nil, check.newInterface will have saved the interface for later completion.
+                       if subst.check == nil { // golang/go#61561: all newly created interfaces must be completed
+                               iface.typeSet()
+                       }
                        return iface
                }