fix: potential deadlock in baton

[DrJosh9000]

Description

Fixes a concurrency design problem in baton.go - revert to using something much closer to @zhming0 's design (#3722).

Currently, the following program deadlocks:

func main() {
	bat := newBaton()

	actor := func(n string) func() {
		return func() {
			time.Sleep(rand.N(time.Microsecond))
			<-bat.Acquire(n)
			time.Sleep(rand.N(time.Microsecond))
			bat.Release(n)
		}
	}

	for {
		var wg sync.WaitGroup
		wg.Go(actor("a"))
		wg.Go(actor("b"))
		wg.Wait()
	}
}

The reason is that Release can occur in between Acquire returning the channel, and the channel receive. Release sees no other actor waiting for the channel, so drops the baton.

Context

It came to me in a dream.

Changes

Revert the design to be closer to #3722

Testing

• Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
• Code is formatted (with go tool gofumpt -extra -w .)

Disclosures / Credits

I did not use AI tools at all

[wolfeidau]

Does this have a test suite which picked this up?

[DrJosh9000]

To answer your question as written: no, there's no existing test suite that picked it up. I don't mind adding a test though.

[zhming0]

The new implementation looks sound so ✅ .

But I have a question about how the deadlock would occur.

[zhming0]

Is this deadlock? or race condition instead.

If release happened in between return ch and <-ch, then I think the worst case is that multiple parties consider themselves as holder at the same time.

But I don't see how deadlock is happening.

[DrJosh9000]

Assume two actors "a" and "b", one runs <-b.Acquire("a") and the other b.Release("b"). Release scans all entries in the b.acquire map to find any that are waiting on the channel. If the Release in "b" happens after Acquire returns (it must at least wait for the mutex), but before the channel is being waited on by "a", then the scan finds no channel being waited on, so assigns the holder to "" and has cleared the map as it goes.