The lock map deleted a mutex someone was waiting on

2026-10-27 · 14 min read

Contents

Assumes you have guarded per-key work in Go with a map of mutexes. Builds on 29 of 34 passing agent runs had a blocker the hidden tests never saw.

This series is about the A9, my 128 GB AMD Strix Halo test box (a GEEKOM A9 Mega: Ryzen AI Max+ 395, 128 GB of soldered LPDDR5X-8000), and lately about one question: can a model running on it, locally, do a developer's job as a coding agent? The study that asks it is a grid: several models, the same ticket-style tasks for each, more than one agent harness, and I grade what each run leaves behind. The grading has several layers, on the theory that any one of them can be fooled and fooling all of them at once is harder. This post is about a run that fooled every layer but one, and I wrote one of the fooled layers myself.

This time the ticket was in Go: add a transfer to a small ledger service. Move money between two accounts, all or nothing, safe under concurrency, and idempotent per key: the client sends a key with each transfer, a retry with the same key must not move the money again, and an attempt that fails must leave no trace, so the client can retry with that key. The ticket even says why it exists: mobile clients that retry on a timeout "have moved the same money twice". Hold on to that sentence. The model is about to rebuild it, with tests.

A scorer grades every run without me. It runs hidden tests the model never sees: core tests for the ticket's rules, and a failure-path suite, added for this second round of the study, whose fake store fails saves on purpose. It plants mutants, small deliberate bugs, in the model's code to check that the model's own tests notice. Runs that score well go to a blind review: a reviewer who doesn't know the model or the harness reads the ticket, the diff and the code, scores the work from 1 to 5, and runs failure-path probes of its own.

The run here is one cell of that grid: one model, one task, one harness, one repetition. The model is Flash-Next Q2_0, Qwen3.8-Flash-Next squeezed into a 62 GiB Q2_0 quantization, served on the A9 by Strata, an inference engine built for that one model family, and driven by opencode, the agent harness that turns the model's replies into file edits and shell commands. The scorer gave it 100 points: 15 of 15 hidden core tests, 7 of 7 mutants, 6 of 6 hidden failure-path tests. I have learned to read 100 points as a dare. The blind review gave it 2 out of 5, "major rework", for one reproduced double move: exactly the bug from the ticket's background section.

The tidiest line in this transfer function is the one that moves the money twice. It's a cleanup. When a call with an idempotency key finishes, the code deletes that key's mutex from its map, so the map doesn't grow forever. Delete that one line and the bug is gone. Keep it, run 16 goroutines that send one transfer with one key against a store that fails 30 % of its saves, and between 226 and 465 of 5,000 rounds end with the money moved more than once.

The cleanup's own comment describes the bug, in its last four words.

The cleanup: unlock the key's mutex, then delete it#

Transfer(ctx, from, to, amount, idempotencyKey) takes a per-key mutex first, then checks whether the key already completed a transfer:

keyMu := s.keyLock(key)
defer func() {
	keyMu.Unlock()
	s.forgetKey(key)
}()
keyMu.Lock()

if done, ok := s.completed(key); ok {
	if done.From == from && done.To == to && done.Amount == amount {
		return done, nil
	}
	return TransferReceipt{}, fmt.Errorf("%w: key %q already transferred %s -> %s %s", ErrIdempotencyConflict, key, done.From, done.To, done.Amount)
}

keyLock hands out the key's mutex from a map, creating it on first use. forgetKey is the cleanup:

// forgetKey drops the in-flight lock of a key once its attempt is over, failed or not. Safe to
// call while the lock is still held: a waiter that already has it replays or retries, and a later
// caller gets a fresh lock and finds the completed record if there is one.
func (s *Service) forgetKey(key string) {
	s.transferMu.Lock()
	defer s.transferMu.Unlock()
	delete(s.inFlight, key)
}

Bug reports usually arrive after the bug. This one shipped with it, as a comment, directly above the function it describes.

"If there is one." Yes. And if there isn't? A completed transfer writes its receipt before the deferred unlock runs, so after a success every later caller finds the record and replays it, whichever mutex it got. The record is missing after a failure, and the ticket requires that: a failed attempt must leave no trace, so the client can retry with the same key. Retrying with the same key is what the key is for.

The account locks in the same file get the other treatment. Their comment says: "Locks are created on first use and live as long as the service; the same id always yields the same mutex." The model wrote the safe version and the tidy version in the same file, with the two comments nine lines apart. Nine lines is not far enough to forget a decision. Apparently it is far enough to make the opposite one.

Three callers, one key, two mutexes#

Caller A takes the key's mutex, call it m1, and starts working. Caller B arrives with the same key, gets m1 from the map and blocks on it. A's save fails. A's deferred function unlocks m1, so B wakes up holding it, and then deletes the map entry. Caller C arrives with the same key, finds no entry, creates m2 and locks it at once. B and C now both hold "the" lock for one key.

That interleaving fits in a 61-line program that imports only sync and fmt. This is its main, with the review's suggested fix as a second variant (more on that below):

func main() {
	for _, cleanup := range []string{"forget", "forgetIfMine"} {
		l := &keyLocks{m: map[string]*sync.Mutex{}}

		a := l.get("k") // attempt A takes the key
		a.Lock()

		b := l.get("k") // attempt B gets the same mutex and waits on it
		bIn := make(chan struct{})
		go func() { b.Lock(); close(bIn) }()

		a.Unlock() // A fails and leaves...
		<-bIn      // ...B is now inside
		if cleanup == "forget" {
			l.forget("k") // ...and A's deferred cleanup runs
		} else {
			l.forgetIfMine("k", a)
		}

		c := l.get("k") // attempt C arrives
		fmt.Printf("%-12s C has B's mutex: %v, C got in while B holds the key: %v\n",
			cleanup, c == b, c.TryLock())
	}
}

It prints the same two lines on every run:

$ go run -race .
forget       C has B's mutex: false, C got in while B holds the key: true
forgetIfMine C has B's mutex: false, C got in while B holds the key: true

TryLock is only the probe here. Its documentation warns that "use of TryLock is often a sign of a deeper problem in a particular use of mutexes", which, for once, is exactly what it found.

Why the second holder moves the money again#

Two callers inside one key's critical section would be harmless if either of them looked again. Neither does. The record check runs once, right after the key lock and before the account locks. B and C both find nothing, because A failed. Then they queue on the two account locks, which the code takes in id order, so there is no deadlock and no lost update. B moves the money, records the receipt and leaves. C passed its check before B wrote anything, so C moves the money again. The account locks stop lost updates between the two of them. Nothing in them knows about keys.

go test -race has nothing to say about any of this, because every map access sits under transferMu. This is a race in the order of events, not in memory.

To pin it down without luck, I wrote a test that holds each save on a channel: A's debit save waits and then fails, B waits on A's key mutex, and C calls only after B is inside its own debit save. One transfer of 10.00 EUR from an account holding 100.00 (balances in cents):

=== RUN   TestLateArrivalAfterFailedAttempt
    latearrival_test.go:52: a=8000 b=2000 (one transfer of 1000 = a=9000 b=1000)
    latearrival_test.go:54: key k moved money 2 times
--- FAIL: TestLateArrivalAfterFailedAttempt (0.10s)

Twenty runs out of twenty, under -race. The review's random version (16 goroutines, one key, 1.00 EUR from 10.00, every save failing with probability 0.3, 5,000 rounds, counting rounds where the destination got more than one transfer) is noisier:

=== RUN   TestProbeI
    probe2_test.go:45: I: iter 1 a=700 b=300
    probe2_test.go:45: I: iter 6 a=700 b=200
    probe2_test.go:45: I: iter 8 a=600 b=200
    probe2_test.go:45: I: iter 9 a=800 b=200
    probe2_test.go:45: I: iter 25 a=800 b=200
    probe2_test.go:49: I: bad 226 / 5000
    probe2_test.go:51: PROBE I REPRODUCED
--- FAIL: TestProbeI (0.95s)

The review counted 439 of 5,000. My 16 runs of the same probe on the delivered code gave 226 to 465; the five on a quiet machine gave 429 to 465, the eleven after other work had pushed the load average to about 10 gave 226 to 443. The count moves with the scheduler. It never reached zero. Sixteen runs, and the best news any of them brought me was 226 rounds with the money moved more than once. Rounds like a=600 b=200 lost money as well: the probe fails compensating saves too, and when the rollback of a debit fails, the code returns an error that says so. That is a separate finding of the same review, reported to the caller, so it did not block.

Checking which mutex you delete doesn't fix it#

The review's finding suggested the fix: "delete only if inFlight[key]==mu, or keep the entry." The first half is the fix I would have written too, and I'd have felt good about it right up to the first probe run:

-		s.forgetKey(key)
+		s.forgetKeyIf(key, keyMu)

func (s *Service) forgetKeyIf(key string, mu *sync.Mutex) {
	s.transferMu.Lock()
	defer s.transferMu.Unlock()
	if s.inFlight[key] == mu {
		delete(s.inFlight, key)
	}
}

It still loses. Three runs of the random probe gave 375, 433 and 429 of 5,000, and the late-arrival test still ends at a=8000 b=2000, twenty runs out of twenty. The forgetIfMine line of the small program says why. When A cleans up, the map entry is A's mutex, so the check passes. It is also B's mutex. The identity check proves the entry belongs to the caller, and B's pointer is a reference the map doesn't count.

Two fixes that hold: keep the entry, or count its users#

The second half of the review's suggestion works: never delete. With the forgetKey call removed, the random probe gave 0 of 5,000 in three runs and the late-arrival test ends at a=9000 b=1000. It costs one mutex per key for the life of the service. This service already keeps every completed receipt for its whole life, and its README says so, so here that cost is a rounding error on a decision already made. The deleted line was saving one small struct per key, and it was paying for that with other people's money.

If the map has to shrink, count who holds or waits for each entry, and delete it only when that count reaches zero, under the same lock that guards the map. The map becomes inFlight map[string]*keyEntry:

// keyEntry is a per-key mutex with the number of callers that hold or wait for it.
type keyEntry struct {
	mu   sync.Mutex
	refs int
}

func (s *Service) keyLock(key string) *sync.Mutex {
	s.transferMu.Lock()
	defer s.transferMu.Unlock()
	e, ok := s.inFlight[key]
	if !ok {
		e = &keyEntry{}
		s.inFlight[key] = e
	}
	e.refs++
	return &e.mu
}

func (s *Service) forgetKey(key string) {
	s.transferMu.Lock()
	defer s.transferMu.Unlock()
	if e := s.inFlight[key]; e != nil {
		e.refs--
		if e.refs == 0 {
			delete(s.inFlight, key)
		}
	}
}

B took its reference in keyLock, before it blocked, so A's cleanup only lowers the count, and C finds the entry and waits behind B. With this change the random probe gave 0 of 5,000 in four runs, the late-arrival test passed twenty of twenty, and nothing else broke under -race: the model's own 24 tests, the 30 hidden core, detail and failure-path tests I could rerun, and the late-arrival test all pass.

Why six of six failure-path tests stayed green#

The double move needs two things in one test: an attempt that fails, and a caller that arrives after that attempt has left while another caller still holds the old mutex. No test in either hidden suite has both at once. I wrote the failure-path suite for bugs exactly like this one, so yes, I am taking the next table personally:

Test	Same key, concurrent	An attempt fails	A late caller while a waiter holds the old mutex
hidden core: concurrent calls with one key	12 goroutines, 20 rounds	no	no
failure path d: one key, two account pairs	8 goroutines, 20 rounds	no	no
failure path e: retry after the second save fails	no, sequential	yes	no
failure path g: waiters released after a failure	1 caller + 3 waiters	yes	no

Without a failure the first attempt records its receipt before it unlocks, and every later caller replays it. The same probe with the failure rate as a knob shows it: at 0 % it finds 0 of 5,000 double moves on the delivered code, at 30 % it finds 476.

Test g comes closest. It holds the first attempt inside its save, lets three more calls with the same key arrive, fails the save, and checks that every call returns within two seconds. Its comment says the balances are checked elsewhere: "How much the waiters moved afterwards is probe d/e territory". In g, all three waiters arrive while the first attempt still holds its save, so they all get the old mutex and take it one at a time; nobody is designed to arrive after the delete. I copied g's setup, cut its pause before the failure from 200 ms to 2 ms, added a balance check and ran it 500 times: zero double moves. Test g did everything right except stand in the doorway at the right moment. A balance check alone would not have caught it. The test needs a fourth caller, timed after the failure.

The model's own same-key test has the same shape as the hidden one: 24 callers, one key, no failing store. It passed every time, for the same reason the hidden tests do. The deadlock test that can't see a half-held lock is the same lesson from the other side: a well-built test whose shape cannot fail against the bug in front of it.

What the README promised#

The README's concurrency paragraph says: "A per-key lock serialises calls that share an idempotency key, so two concurrent calls with the same key move the money exactly once — the second one finds the recorded receipt." The closing summary says the per-key lock "makes same-key concurrent calls move money once". The blind review labelled that claim an overclaim: the lock is there and it is wrong, which under the honesty rule is a bug, not a lie. The summary also counts "8 tests" in a test file with ten. An undercount. After this series, I'll take it.

The comment was nearly right. "A later caller gets a fresh lock and finds the completed record if there is one" is true on every path where there is one. The ticket's retry rule guarantees the path where there isn't, and the fix for that path is to delete one line.

Every time I reread that comment, I want to answer its last four words in the margin: and if there isn't, a second transfer, in 226 to 465 rounds of 5,000. I've resisted. The code under it already says so, in euros, to anyone who retries while someone else is still waiting on the key.

Filed under: go, concurrency, testing, ai-agents, local-llm

Sources#

Go sync package documentation: Mutex.TryLock and its note on rare correct uses.
Benchmark artifacts (local, not published): the cell's delivered code, score row, blind review and honesty check; copies of the hidden core and failure-path suites; my probe, late-arrival and variant test files and their logs, and the 61-line standalone program (Go 1.26.0, run 2026-10-27).