Skip to content

Commit 7b53d8d

Browse files
Mark FreemanGo LUCI
authored andcommitted
cmd/compile/internal/types2: add loaded state between loader calls and constraint expansion
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 <[email protected]> Reviewed-by: Robert Griesemer <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 374e3be commit 7b53d8d

File tree

7 files changed

+126
-30
lines changed

7 files changed

+126
-30
lines changed

src/cmd/compile/internal/importer/gcimporter_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,3 +673,50 @@ type S struct {
673673
}
674674
wg.Wait()
675675
}
676+
677+
func TestIssue63285(t *testing.T) {
678+
testenv.MustHaveGoBuild(t)
679+
680+
// This package only handles gc export data.
681+
if runtime.Compiler != "gc" {
682+
t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
683+
}
684+
685+
tmpdir := t.TempDir()
686+
testoutdir := filepath.Join(tmpdir, "testdata")
687+
if err := os.Mkdir(testoutdir, 0700); err != nil {
688+
t.Fatalf("making output dir: %v", err)
689+
}
690+
691+
compile(t, "testdata", "issue63285.go", testoutdir, nil)
692+
693+
issue63285, err := Import(make(map[string]*types2.Package), "./testdata/issue63285", tmpdir, nil)
694+
if err != nil {
695+
t.Fatal(err)
696+
}
697+
698+
check := func(pkgname, src string, imports importMap) (*types2.Package, error) {
699+
f, err := syntax.Parse(syntax.NewFileBase(pkgname), strings.NewReader(src), nil, nil, 0)
700+
if err != nil {
701+
return nil, err
702+
}
703+
config := &types2.Config{
704+
Importer: imports,
705+
}
706+
return config.Check(pkgname, []*syntax.File{f}, nil)
707+
}
708+
709+
const pSrc = `package p
710+
711+
import "issue63285"
712+
713+
var _ issue63285.A[issue63285.B[any]]
714+
`
715+
716+
importer := importMap{
717+
"issue63285": issue63285,
718+
}
719+
if _, err := check("p", pSrc, importer); err != nil {
720+
t.Errorf("Check failed: %v", err)
721+
}
722+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package issue63285
6+
7+
type A[_ B[any]] struct{}
8+
9+
type B[_ any] interface {
10+
f() A[B[any]]
11+
}

src/cmd/compile/internal/importer/ureader.go

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ type reader struct {
6767

6868
p *pkgReader
6969

70-
dict *readerDict
70+
dict *readerDict
71+
delayed []func()
7172
}
7273

7374
type readerDict struct {
@@ -420,7 +421,7 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index) (*types2.Package, string) {
420421
pos := r.pos()
421422
var tparams []*types2.TypeParam
422423
if r.Version().Has(pkgbits.AliasTypeParamNames) {
423-
tparams = r.typeParamNames()
424+
tparams = r.typeParamNames(false)
424425
}
425426
typ := r.typ()
426427
return newAliasTypeName(pr.enableAlias, pos, objPkg, objName, typ, tparams)
@@ -433,28 +434,28 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index) (*types2.Package, string) {
433434

434435
case pkgbits.ObjFunc:
435436
pos := r.pos()
436-
tparams := r.typeParamNames()
437+
tparams := r.typeParamNames(false)
437438
sig := r.signature(nil, nil, tparams)
438439
return types2.NewFunc(pos, objPkg, objName, sig)
439440

440441
case pkgbits.ObjType:
441442
pos := r.pos()
442443

443-
return types2.NewTypeNameLazy(pos, objPkg, objName, func(named *types2.Named) (tparams []*types2.TypeParam, underlying types2.Type, methods []*types2.Func) {
444-
tparams = r.typeParamNames()
444+
return types2.NewTypeNameLazy(pos, objPkg, objName, func(_ *types2.Named) ([]*types2.TypeParam, types2.Type, []*types2.Func, []func()) {
445+
tparams := r.typeParamNames(true)
445446

446447
// TODO(mdempsky): Rewrite receiver types to underlying is an
447448
// Interface? The go/types importer does this (I think because
448449
// unit tests expected that), but cmd/compile doesn't care
449450
// about it, so maybe we can avoid worrying about that here.
450-
underlying = r.typ().Underlying()
451+
underlying := r.typ().Underlying()
451452

452-
methods = make([]*types2.Func, r.Len())
453+
methods := make([]*types2.Func, r.Len())
453454
for i := range methods {
454-
methods[i] = r.method()
455+
methods[i] = r.method(true)
455456
}
456457

457-
return
458+
return tparams, underlying, methods, r.delayed
458459
})
459460

460461
case pkgbits.ObjVar:
@@ -497,7 +498,7 @@ func (pr *pkgReader) objDictIdx(idx pkgbits.Index) *readerDict {
497498
return &dict
498499
}
499500

500-
func (r *reader) typeParamNames() []*types2.TypeParam {
501+
func (r *reader) typeParamNames(isLazy bool) []*types2.TypeParam {
501502
r.Sync(pkgbits.SyncTypeParamNames)
502503

503504
// Note: This code assumes it only processes objects without
@@ -523,19 +524,38 @@ func (r *reader) typeParamNames() []*types2.TypeParam {
523524
r.dict.tparams[i] = types2.NewTypeParam(tname, nil)
524525
}
525526

526-
for i, bound := range r.dict.bounds {
527-
r.dict.tparams[i].SetConstraint(r.p.typIdx(bound, r.dict))
527+
// Type parameters that are read by lazy loaders cannot have their
528+
// constraints set eagerly; do them after loading (go.dev/issue/63285).
529+
if isLazy {
530+
// The reader dictionary will continue mutating before we have time
531+
// to call delayed functions; must make a local copy of both the type
532+
// parameters and their (unexpanded) constraints.
533+
bounds := make([]types2.Type, len(r.dict.bounds))
534+
for i, bound := range r.dict.bounds {
535+
bounds[i] = r.p.typIdx(bound, r.dict)
536+
}
537+
538+
tparams := r.dict.tparams
539+
r.delayed = append(r.delayed, func() {
540+
for i, bound := range bounds {
541+
tparams[i].SetConstraint(bound)
542+
}
543+
})
544+
} else {
545+
for i, bound := range r.dict.bounds {
546+
r.dict.tparams[i].SetConstraint(r.p.typIdx(bound, r.dict))
547+
}
528548
}
529549

530550
return r.dict.tparams
531551
}
532552

533-
func (r *reader) method() *types2.Func {
553+
func (r *reader) method(isLazy bool) *types2.Func {
534554
r.Sync(pkgbits.SyncMethod)
535555
pos := r.pos()
536556
pkg, name := r.selector()
537557

538-
rtparams := r.typeParamNames()
558+
rtparams := r.typeParamNames(isLazy)
539559
sig := r.signature(r.param(), rtparams, nil)
540560

541561
_ = r.pos() // TODO(mdempsky): Remove; this is a hacker for linker.go.

src/cmd/compile/internal/types2/named.go

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,8 @@ type Named struct {
127127
// accessed.
128128
methods []*Func
129129

130-
// loader may be provided to lazily load type parameters, underlying type, and methods.
131-
loader func(*Named) (tparams []*TypeParam, underlying Type, methods []*Func)
130+
// loader may be provided to lazily load type parameters, underlying type, methods, and delayed functions
131+
loader func(*Named) ([]*TypeParam, Type, []*Func, []func())
132132
}
133133

134134
// instance holds information that is only necessary for instantiated named
@@ -143,9 +143,11 @@ type instance struct {
143143
// namedState represents the possible states that a named type may assume.
144144
type namedState uint32
145145

146+
// Note: the order of states is relevant
146147
const (
147148
unresolved namedState = iota // tparams, underlying type and methods might be unavailable
148-
resolved // resolve has run; methods might be incomplete (for instances)
149+
resolved // resolve has run; methods might be unexpanded (for instances)
150+
loaded // loader has run; constraints might be unexpanded (for generic types)
149151
complete // all data is known
150152
)
151153

@@ -167,7 +169,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named {
167169
// accessible; but if n is an instantiated type, its methods may still be
168170
// unexpanded.
169171
func (n *Named) resolve() *Named {
170-
if n.state() >= resolved { // avoid locking below
172+
if n.state() > unresolved { // avoid locking below
171173
return n
172174
}
173175

@@ -176,7 +178,7 @@ func (n *Named) resolve() *Named {
176178
n.mu.Lock()
177179
defer n.mu.Unlock()
178180

179-
if n.state() >= resolved {
181+
if n.state() > unresolved {
180182
return n
181183
}
182184

@@ -212,13 +214,20 @@ func (n *Named) resolve() *Named {
212214
assert(n.underlying == nil)
213215
assert(n.TypeArgs().Len() == 0) // instances are created by instantiation, in which case n.loader is nil
214216

215-
tparams, underlying, methods := n.loader(n)
217+
tparams, underlying, methods, delayed := n.loader(n)
218+
n.loader = nil
216219

217220
n.tparams = bindTParams(tparams)
218221
n.underlying = underlying
219222
n.fromRHS = underlying // for cycle detection
220223
n.methods = methods
221-
n.loader = nil
224+
225+
// advance state to avoid deadlock calling delayed functions
226+
n.setState(loaded)
227+
228+
for _, f := range delayed {
229+
f()
230+
}
222231
}
223232

224233
n.setState(complete)

src/cmd/compile/internal/types2/object.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ func NewTypeName(pos syntax.Pos, pkg *Package, name string, typ Type) *TypeName
293293

294294
// NewTypeNameLazy returns a new defined type like NewTypeName, but it
295295
// lazily calls resolve to finish constructing the Named object.
296-
func NewTypeNameLazy(pos syntax.Pos, pkg *Package, name string, load func(named *Named) (tparams []*TypeParam, underlying Type, methods []*Func)) *TypeName {
296+
func NewTypeNameLazy(pos syntax.Pos, pkg *Package, name string, load func(*Named) ([]*TypeParam, Type, []*Func, []func())) *TypeName {
297297
obj := NewTypeName(pos, pkg, name, nil)
298298
NewNamed(obj, nil, nil).loader = load
299299
return obj

src/go/types/named.go

Lines changed: 16 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/go/types/object.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)