tree 58dfc3ce09a197549248dddc492d5232f129cb0a
parent 8e070db38e5c55da6a85c81878ab769bf5667848
author Bryan C. Mills <bcmills@google.com> 1524600329 -0400
committer Bryan C. Mills <bcmills@google.com> 1525119475 +0000

imports: fix races in findImportGoPath

Before this change, findImportGoPath used a field within the
(otherwise read-only) structs in the dirScan map to cache the distance
from the importing package to the candidate package to be imported. As
a result, the top-level imports.Process function was not safe to call
concurrently: one goroutine could overwrite the distances while
another was attempting to sort by them. Furthermore, there were some
internal write-after-write races (writing the same cached distance to
the same address) that otherwise violate the Go memory model.

This change fixes those races, simplifies the concurrency patterns,
and clarifies goroutine lifetimes. The functions in the imports
package now wait for the goroutines they spawn to finish before
returning, eliminating the need for an awkward test-only mutex that
could otherwise mask real races in the production code paths.

See also:
https://golang.org/wiki/CodeReviewComments#goroutine-lifetimes
https://golang.org/wiki/CodeReviewComments#synchronous-functions

Fixes golang/go#25030.

Change-Id: I8fec735e0d4ff7abab406dea9d0c11d1bd93d775
Reviewed-on: https://go-review.googlesource.com/109156
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>