Fix lost-wakeup race on packageWaits in WaitForPackage

src/core/state.go

@@ -884,14 +884,16 @@ func (state *BuildState) WaitForPackage(l, dependent BuildLabel, mode ParseMode)
 	}
 
 	// If something has already queued the package to be parsed, wait for them
-	if ch := state.progress.packageWaits.Get(key); ch != nil {
+	// (atomically: a racing Get-then-Set here can orphan the first caller's channel)
+	if ch, inserted := state.progress.packageWaits.AddOrGet(key, func() chan struct{} {
+		return make(chan struct{})
+	}); !inserted {
 		waitOnChan(ch, "Still waiting for package wait in WaitForPackage(%v, %v, %v)", l, dependent, mode)
 		return state.Graph.PackageByLabel(l)
 	}
 
 	// Otherwise queue the target for parse and recurse
 	state.addPendingParse(l, dependent, mode)
-	state.progress.packageWaits.Set(key, make(chan struct{}))
 
 	return state.WaitForPackage(l, dependent, mode)
 }

src/core/state_test.go

@@ -2,7 +2,9 @@ package core
 
 import (
 	"strings"
+	"sync"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -167,3 +169,39 @@ func TestCopyPlugin(t *testing.T) {
 
 	assert.NotEqual(t, plugin.ExtraValues["foo"], newPlugin.ExtraValues["foo"])
 }
+
+func TestWaitForPackageConcurrent(t *testing.T) {
+	// Regression test for a lost-wakeup race: concurrent callers waiting on
+	// the same unparsed package could overwrite each other's wait channel in
+	// packageWaits, so the channel one of them waited on was never closed and
+	// that caller blocked forever.
+	dependent := BuildLabel{PackageName: "other", Name: "all"}
+	for i := 0; i < 200; i++ {
+		state := NewDefaultBuildState()
+		label := BuildLabel{PackageName: "pkg", Name: "all"}
+		const n = 32
+		var wg sync.WaitGroup
+		wg.Add(n)
+		start := make(chan struct{})
+		for j := 0; j < n; j++ {
+			go func() {
+				defer wg.Done()
+				<-start
+				state.WaitForPackage(label, dependent, ParseModeNormal)
+			}()
+		}
+		close(start)
+		// Let the waiters register against the unparsed package first, then
+		// complete the parse the way LogParseResult does for real parses.
+		time.Sleep(time.Millisecond)
+		state.Graph.AddPackage(NewPackage("pkg"))
+		state.LogParseResult(label, PackageParsed, "parsed")
+		done := make(chan struct{})
+		go func() { wg.Wait(); close(done) }()
+		select {
+		case <-done:
+		case <-time.After(10 * time.Second):
+			t.Fatalf("iteration %d: a WaitForPackage caller never woke", i)
+		}
+	}
+}