]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile/internal/types2: add loaded state between loader calls and constraint...
authorMark Freeman <mark@golang.org>
Fri, 13 Jun 2025 15:47:57 +0000 (11:47 -0400)
committerGo LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Fri, 25 Jul 2025 20:42:54 +0000 (13:42 -0700)
There is a deadlock issue when calling SetConstraint from a lazy loader
because the loader is called from resolve(), which is holding a lock on
the loaded type.

If the loaded type has a generic constraint which refers back to the
loaded type (such as an argument or result), then we will loop back to
the loaded type and deadlock.

This change postpones calls to SetConstraint and passes them back to
resolve(). At that point, the loaded type is mostly constructed, but
its constraints might be unexpanded.

Similar to how we handle resolved instances, we advance the state for
the loaded type to a, appropriately named, loaded state. When we expand
the constraint, we don't try to acquire the lock on the loaded type.
Thus, no deadlock.

Fixes #63285

Change-Id: Ie0204b58a5b433f6d839ce8fd8a99542246367b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/681875
Commit-Queue: Mark Freeman <mark@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/cmd/compile/internal/importer/gcimporter_test.go
src/cmd/compile/internal/importer/testdata/issue63285.go [new file with mode: 0644]
src/cmd/compile/internal/importer/ureader.go
src/cmd/compile/internal/types2/named.go
src/cmd/compile/internal/types2/object.go
src/go/types/named.go
src/go/types/object.go

index 965c5d1a84b3c0550a73262b0c80aaa1b1b8e183..11e4ee6b583b6af839dd7e4938253a4891994c98 100644 (file)
@@ -673,3 +673,50 @@ type S struct {
        }
        wg.Wait()
 }
+
+func TestIssue63285(t *testing.T) {
+       testenv.MustHaveGoBuild(t)
+
+       // This package only handles gc export data.
+       if runtime.Compiler != "gc" {
+               t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
+       }
+
+       tmpdir := t.TempDir()
+       testoutdir := filepath.Join(tmpdir, "testdata")
+       if err := os.Mkdir(testoutdir, 0700); err != nil {
+               t.Fatalf("making output dir: %v", err)
+       }
+
+       compile(t, "testdata", "issue63285.go", testoutdir, nil)
+
+       issue63285, err := Import(make(map[string]*types2.Package), "./testdata/issue63285", tmpdir, nil)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       check := func(pkgname, src string, imports importMap) (*types2.Package, error) {
+               f, err := syntax.Parse(syntax.NewFileBase(pkgname), strings.NewReader(src), nil, nil, 0)
+               if err != nil {
+                       return nil, err
+               }
+               config := &types2.Config{
+                       Importer: imports,
+               }
+               return config.Check(pkgname, []*syntax.File{f}, nil)
+       }
+
+       const pSrc = `package p
+
+import "issue63285"
+
+var _ issue63285.A[issue63285.B[any]]
+`
+
+       importer := importMap{
+               "issue63285": issue63285,
+       }
+       if _, err := check("p", pSrc, importer); err != nil {
+               t.Errorf("Check failed: %v", err)
+       }
+}
diff --git a/src/cmd/compile/internal/importer/testdata/issue63285.go b/src/cmd/compile/internal/importer/testdata/issue63285.go
new file mode 100644 (file)
index 0000000..54a06b7
--- /dev/null
@@ -0,0 +1,11 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package issue63285
+
+type A[_ B[any]] struct{}
+
+type B[_ any] interface {
+       f() A[B[any]]
+}
index 6012d283ac5731b9ab02c7c25e756b2e2ab3ecb4..f6df56b70e665c4efdf9c942405187c8f396e839 100644 (file)
@@ -67,7 +67,8 @@ type reader struct {
 
        p *pkgReader
 
-       dict *readerDict
+       dict    *readerDict
+       delayed []func()
 }
 
 type readerDict struct {
@@ -420,7 +421,7 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index) (*types2.Package, string) {
                        pos := r.pos()
                        var tparams []*types2.TypeParam
                        if r.Version().Has(pkgbits.AliasTypeParamNames) {
-                               tparams = r.typeParamNames()
+                               tparams = r.typeParamNames(false)
                        }
                        typ := r.typ()
                        return newAliasTypeName(pr.enableAlias, pos, objPkg, objName, typ, tparams)
@@ -433,28 +434,28 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index) (*types2.Package, string) {
 
                case pkgbits.ObjFunc:
                        pos := r.pos()
-                       tparams := r.typeParamNames()
+                       tparams := r.typeParamNames(false)
                        sig := r.signature(nil, nil, tparams)
                        return types2.NewFunc(pos, objPkg, objName, sig)
 
                case pkgbits.ObjType:
                        pos := r.pos()
 
-                       return types2.NewTypeNameLazy(pos, objPkg, objName, func(named *types2.Named) (tparams []*types2.TypeParam, underlying types2.Type, methods []*types2.Func) {
-                               tparams = r.typeParamNames()
+                       return types2.NewTypeNameLazy(pos, objPkg, objName, func(_ *types2.Named) ([]*types2.TypeParam, types2.Type, []*types2.Func, []func()) {
+                               tparams := r.typeParamNames(true)
 
                                // TODO(mdempsky): Rewrite receiver types to underlying is an
                                // Interface? The go/types importer does this (I think because
                                // unit tests expected that), but cmd/compile doesn't care
                                // about it, so maybe we can avoid worrying about that here.
-                               underlying = r.typ().Underlying()
+                               underlying := r.typ().Underlying()
 
-                               methods = make([]*types2.Func, r.Len())
+                               methods := make([]*types2.Func, r.Len())
                                for i := range methods {
-                                       methods[i] = r.method()
+                                       methods[i] = r.method(true)
                                }
 
-                               return
+                               return tparams, underlying, methods, r.delayed
                        })
 
                case pkgbits.ObjVar:
@@ -497,7 +498,7 @@ func (pr *pkgReader) objDictIdx(idx pkgbits.Index) *readerDict {
        return &dict
 }
 
-func (r *reader) typeParamNames() []*types2.TypeParam {
+func (r *reader) typeParamNames(isLazy bool) []*types2.TypeParam {
        r.Sync(pkgbits.SyncTypeParamNames)
 
        // Note: This code assumes it only processes objects without
@@ -523,19 +524,38 @@ func (r *reader) typeParamNames() []*types2.TypeParam {
                r.dict.tparams[i] = types2.NewTypeParam(tname, nil)
        }
 
-       for i, bound := range r.dict.bounds {
-               r.dict.tparams[i].SetConstraint(r.p.typIdx(bound, r.dict))
+       // Type parameters that are read by lazy loaders cannot have their
+       // constraints set eagerly; do them after loading (go.dev/issue/63285).
+       if isLazy {
+               // The reader dictionary will continue mutating before we have time
+               // to call delayed functions; must make a local copy of both the type
+               // parameters and their (unexpanded) constraints.
+               bounds := make([]types2.Type, len(r.dict.bounds))
+               for i, bound := range r.dict.bounds {
+                       bounds[i] = r.p.typIdx(bound, r.dict)
+               }
+
+               tparams := r.dict.tparams
+               r.delayed = append(r.delayed, func() {
+                       for i, bound := range bounds {
+                               tparams[i].SetConstraint(bound)
+                       }
+               })
+       } else {
+               for i, bound := range r.dict.bounds {
+                       r.dict.tparams[i].SetConstraint(r.p.typIdx(bound, r.dict))
+               }
        }
 
        return r.dict.tparams
 }
 
-func (r *reader) method() *types2.Func {
+func (r *reader) method(isLazy bool) *types2.Func {
        r.Sync(pkgbits.SyncMethod)
        pos := r.pos()
        pkg, name := r.selector()
 
-       rtparams := r.typeParamNames()
+       rtparams := r.typeParamNames(isLazy)
        sig := r.signature(r.param(), rtparams, nil)
 
        _ = r.pos() // TODO(mdempsky): Remove; this is a hacker for linker.go.
index a9a27c93206c3685c21a8a8a2bd293e9329718c8..dbb1fa0b3e75f002af8ec6eb31d4f4093afc0c0b 100644 (file)
@@ -127,8 +127,8 @@ type Named struct {
        // accessed.
        methods []*Func
 
-       // loader may be provided to lazily load type parameters, underlying type, and methods.
-       loader func(*Named) (tparams []*TypeParam, underlying Type, methods []*Func)
+       // loader may be provided to lazily load type parameters, underlying type, methods, and delayed functions
+       loader func(*Named) ([]*TypeParam, Type, []*Func, []func())
 }
 
 // instance holds information that is only necessary for instantiated named
@@ -143,9 +143,11 @@ type instance struct {
 // namedState represents the possible states that a named type may assume.
 type namedState uint32
 
+// Note: the order of states is relevant
 const (
        unresolved namedState = iota // tparams, underlying type and methods might be unavailable
-       resolved                     // resolve has run; methods might be incomplete (for instances)
+       resolved                     // resolve has run; methods might be unexpanded (for instances)
+       loaded                       // loader has run; constraints might be unexpanded (for generic types)
        complete                     // all data is known
 )
 
@@ -167,7 +169,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named {
 // accessible; but if n is an instantiated type, its methods may still be
 // unexpanded.
 func (n *Named) resolve() *Named {
-       if n.state() >resolved { // avoid locking below
+       if n.state() > unresolved { // avoid locking below
                return n
        }
 
@@ -176,7 +178,7 @@ func (n *Named) resolve() *Named {
        n.mu.Lock()
        defer n.mu.Unlock()
 
-       if n.state() >resolved {
+       if n.state() > unresolved {
                return n
        }
 
@@ -212,13 +214,20 @@ func (n *Named) resolve() *Named {
                assert(n.underlying == nil)
                assert(n.TypeArgs().Len() == 0) // instances are created by instantiation, in which case n.loader is nil
 
-               tparams, underlying, methods := n.loader(n)
+               tparams, underlying, methods, delayed := n.loader(n)
+               n.loader = nil
 
                n.tparams = bindTParams(tparams)
                n.underlying = underlying
                n.fromRHS = underlying // for cycle detection
                n.methods = methods
-               n.loader = nil
+
+               // advance state to avoid deadlock calling delayed functions
+               n.setState(loaded)
+
+               for _, f := range delayed {
+                       f()
+               }
        }
 
        n.setState(complete)
index 26752c44b0655874f10fe489c6b19e90a2c2dfd5..7096c556971077643271ca37c0cd00b0367b1bed 100644 (file)
@@ -293,7 +293,7 @@ func NewTypeName(pos syntax.Pos, pkg *Package, name string, typ Type) *TypeName
 
 // NewTypeNameLazy returns a new defined type like NewTypeName, but it
 // lazily calls resolve to finish constructing the Named object.
-func NewTypeNameLazy(pos syntax.Pos, pkg *Package, name string, load func(named *Named) (tparams []*TypeParam, underlying Type, methods []*Func)) *TypeName {
+func NewTypeNameLazy(pos syntax.Pos, pkg *Package, name string, load func(*Named) ([]*TypeParam, Type, []*Func, []func())) *TypeName {
        obj := NewTypeName(pos, pkg, name, nil)
        NewNamed(obj, nil, nil).loader = load
        return obj
index 1282abfa3f078a1889b31c4214ee1fb3f801bfc0..72fd4970bb33840340f0dfb05ac83dd6cb647dbf 100644 (file)
@@ -130,8 +130,8 @@ type Named struct {
        // accessed.
        methods []*Func
 
-       // loader may be provided to lazily load type parameters, underlying type, and methods.
-       loader func(*Named) (tparams []*TypeParam, underlying Type, methods []*Func)
+       // loader may be provided to lazily load type parameters, underlying type, methods, and delayed functions
+       loader func(*Named) ([]*TypeParam, Type, []*Func, []func())
 }
 
 // instance holds information that is only necessary for instantiated named
@@ -146,9 +146,11 @@ type instance struct {
 // namedState represents the possible states that a named type may assume.
 type namedState uint32
 
+// Note: the order of states is relevant
 const (
        unresolved namedState = iota // tparams, underlying type and methods might be unavailable
-       resolved                     // resolve has run; methods might be incomplete (for instances)
+       resolved                     // resolve has run; methods might be unexpanded (for instances)
+       loaded                       // loader has run; constraints might be unexpanded (for generic types)
        complete                     // all data is known
 )
 
@@ -170,7 +172,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named {
 // accessible; but if n is an instantiated type, its methods may still be
 // unexpanded.
 func (n *Named) resolve() *Named {
-       if n.state() >resolved { // avoid locking below
+       if n.state() > unresolved { // avoid locking below
                return n
        }
 
@@ -179,7 +181,7 @@ func (n *Named) resolve() *Named {
        n.mu.Lock()
        defer n.mu.Unlock()
 
-       if n.state() >resolved {
+       if n.state() > unresolved {
                return n
        }
 
@@ -215,13 +217,20 @@ func (n *Named) resolve() *Named {
                assert(n.underlying == nil)
                assert(n.TypeArgs().Len() == 0) // instances are created by instantiation, in which case n.loader is nil
 
-               tparams, underlying, methods := n.loader(n)
+               tparams, underlying, methods, delayed := n.loader(n)
+               n.loader = nil
 
                n.tparams = bindTParams(tparams)
                n.underlying = underlying
                n.fromRHS = underlying // for cycle detection
                n.methods = methods
-               n.loader = nil
+
+               // advance state to avoid deadlock calling delayed functions
+               n.setState(loaded)
+
+               for _, f := range delayed {
+                       f()
+               }
        }
 
        n.setState(complete)
index aa7dcb835c6cc86f13f214569728affa45afd605..823c03c7fd94ce34e68bb2ccece8102bc38a87ab 100644 (file)
@@ -296,7 +296,7 @@ func NewTypeName(pos token.Pos, pkg *Package, name string, typ Type) *TypeName {
 
 // NewTypeNameLazy returns a new defined type like NewTypeName, but it
 // lazily calls resolve to finish constructing the Named object.
-func _NewTypeNameLazy(pos token.Pos, pkg *Package, name string, load func(named *Named) (tparams []*TypeParam, underlying Type, methods []*Func)) *TypeName {
+func _NewTypeNameLazy(pos token.Pos, pkg *Package, name string, load func(*Named) ([]*TypeParam, Type, []*Func, []func())) *TypeName {
        obj := NewTypeName(pos, pkg, name, nil)
        NewNamed(obj, nil, nil).loader = load
        return obj