1
0
mirror of https://github.com/golang/go synced 2024-10-01 03:28:32 -06:00

go/packages: fix race detected by race detector

Diamonds in the dependency graph could cause the same package
to be processed by two separate gorotines, causing a potential
race between checking if the package's Types field is set, and
setting the field.

adonovan's comment suggested this might be unnecessary. We'll
ask him about this once he returns from his vacation.

Change-Id: I27511b6e44bb1b6a04ea294206fd8ed4ea60f56f
Reviewed-on: https://go-review.googlesource.com/127636
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit is contained in:
Michael Matloob 2018-08-02 15:35:27 -04:00
parent 4a42e0a488
commit 82515cf895

View File

@ -505,6 +505,10 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) {
// used to supply alternative file contents.
return nil, fmt.Errorf("no metadata for %s", path)
}
ld.exportMu.Lock()
defer ld.exportMu.Unlock()
if ipkg.Types != nil && ipkg.Types.Complete() {
return ipkg.Types, nil
}
@ -629,8 +633,16 @@ func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error
// Not all accesses to Package.Pkg need to be protected by exportMu:
// graph ordering ensures that direct dependencies of source
// packages are fully loaded before the importer reads their Pkg field.
ld.exportMu.Lock()
defer ld.exportMu.Unlock()
//
// TODO(matloob): Ask adonovan if this is safe. Because of diamonds in the
// dependency graph, it's possible that the same package is loaded in
// two separate goroutines and there's a race in the write of the Package's
// Types here and the read earlier checking if types is set before calling
// this function.
/*
ld.exportMu.Lock()
defer ld.exportMu.Unlock()
*/
if tpkg := lpkg.Types; tpkg != nil && tpkg.Complete() {
return tpkg, nil // cache hit