What interleaving still drops a task when Go's Submit checks a closing flag and then calls wg.Add(1)?

middleshould knowfreq 38%

answer

two atomic steps are not one atomic step
something happens between the check and the count
shutdown can slip into the gap
the write lock waits for half-done admissions
the race detector never sees this one

basics

~20 s

Submit reads the flag as false, then shutdown sets it and finishes waiting at a zero counter before Submit's Add runs. The task is admitted after the drain ended. Check and count must be one indivisible step.

solid answer

~50 s

The two steps are individually atomic but not atomic together, so there is a window between them. Submit reads closing as false; before it reaches wg.Add(1), the shutdown goroutine sets closing, calls wg.Wait(), finds the counter at zero and returns; Submit then increments the counter and starts work that nobody is waiting for. That is both a lost record and a documented sync misuse, since the Add lifts the counter off zero while a Wait is in progress. The fix is to make admission a single critical section: Submit takes a sync.RWMutex read lock, checks the flag, calls wg.Add(1) and releases; Shutdown takes the write lock, sets the flag, releases, then waits. Because the write lock cannot be taken while any Submit is mid-admission, once it is released every future Submit sees the closed flag.

The two steps are individually atomic but not atomic together, so there is a window between them. Submit reads `closing` as false; before it reaches `wg.Add(1)`, the shutdown goroutine sets `closing`, calls `wg.Wait()`, finds the counter at zero and returns; Submit then increments the counter and starts work that nobody is waiting for. That is both a lost record and a documented `sync` misuse, since the `Add` lifts the counter off zero while a `Wait` is in progress. The fix is to make admission a single critical section: `Submit` takes a `sync.RWMutex` read lock, checks the flag, calls `wg.Add(1)` and releases; `Shutdown` takes the write lock, sets the flag, releases, then waits. Because the write lock cannot be taken while any `Submit` is mid-admission, once it is released every future `Submit` sees the closed flag.

code

go · 26 lines

type Writer struct {
	mu      sync.RWMutex
	closing bool // guarded by mu; no longer needs to be atomic
	wg      sync.WaitGroup
}

func (w *Writer) Submit(r Record) error {
	w.mu.RLock()
	defer w.mu.RUnlock()
	if w.closing {
		return errors.New("audit writer is draining")
	}
	w.wg.Add(1) // check and count are one indivisible step
	go func() {
		defer w.wg.Done()
		w.write(r)
	}()
	return nil
}

func (w *Writer) Shutdown() {
	w.mu.Lock()
	w.closing = true
	w.mu.Unlock() // no admission is mid-flight past this point
	w.wg.Wait()
}

go deeper

for a junior

Know that a shutdown flag and a counter are two separate steps, and that whatever happens between them is where work gets lost. Be able to say the fix is to do both under one lock.

for a middle

Draw the interleaving line by line and say which goroutine does what at each step. Explain why an atomic flag does not help and why the write lock is what guarantees no admission is half-done.

for a senior

Weigh the fixes: a read lock on the accept path, a compare-and-swap counter with a closed sentinel, or a token channel. Say what each costs on a hot path and when accepting the window is a defensible decision rather than a bug.

for a principal

Decide what an accepted-then-lost item is worth to the business and let that set the design: an idempotent, retried workload may tolerate the window, while an audit trail may not, and that judgement should be written down beside the code.

The window

A drain built as "check a flag, then count the item" has two operations that are each atomic on their own — an atomic load and a WaitGroup increment — but nothing makes the pair atomic. Between them the goroutine can be preempted for an arbitrary length of time. Write out the interleaving explicitly:

step	submitting goroutine	shutdown goroutine
1	reads closing flag → false
2	(descheduled)	sets closing flag → true
3		calls Wait; counter is 0; returns
4		closes the output, exits
5	increments counter, starts work

At step 5 the record has been admitted by a service that already told the world it was drained. The work either never runs, or runs against a closed output, or is cut short by process exit. From the caller's perspective Submit returned nil and the record vanished, which is the single worst shutdown failure mode for something like an audit-log writer.

There is a second problem hiding at step 5. sync.WaitGroup requires that an Add which raises the counter from zero happen before any Wait on that group; the reuse case is only safe when the previous Wait has fully returned. Step 5 races that rule directly. sync detects some of these and panics reporting WaitGroup misuse, so the bug can surface as a crash rather than as a quietly dropped record — which is, perversely, the luckier outcome.

Note what the race detector will and will not do here. Both accesses to the flag are properly synchronised, so -race sees no data race on it. The defect is a logical race in the admission protocol, and only the WaitGroup's own misuse check has any chance of catching it. You find this one by reasoning about the interleaving, not by running the detector.

Making admission one step

The requirement is precise: after the shutdown goroutine has published "closed", there must be no goroutine anywhere between the check and the increment. A reader-writer lock expresses exactly that.

Submit takes the read lock, checks the flag, and — still holding it — calls wg.Add(1), then releases. Shutdown takes the write lock, sets the flag, releases it, and only then calls wg.Wait(). The write lock cannot be acquired while any reader is inside its critical section, so acquiring it means every in-progress admission has completed its Add; and releasing it means every future admission will observe the flag as set. The counter is therefore at its final maximum before Wait is ever called, and can only fall from there.

A read lock is the right shape rather than a plain mutex because admissions are concurrent with each other and only the single shutdown needs exclusivity. The critical section is tiny — a bool read and an integer increment — so the added contention is negligible next to the work being admitted. A plain sync.Mutex is perfectly acceptable too and is simpler to reason about; use it if admissions are not hot.

Once the flag is under the lock, it no longer needs to be an atomic type at all. Keeping both an atomic.Bool and a mutex is a common half-fix: the atomic makes the individual read safe, which was never the problem.

Variants you will see

One counter, with a closed sentinel. Instead of a flag plus a WaitGroup, keep a single atomic counter and admit with a compare-and-swap loop that refuses whenever a "closed" bit is set. Admission and counting become literally one instruction, so the window cannot exist. Shutdown sets the bit and then waits for the count to reach zero, usually by having the last Done close a channel. It is more code and easier to get wrong, but it avoids a lock on a genuinely hot path.

A token channel. Give the service a buffered channel of tokens sized to the concurrency limit, and admit only if a token can be taken and a closing channel is not yet closed, expressed as one select. Shutdown closes closing and then reclaims every token. Admission is again a single atomic decision, and the same structure gives you a concurrency cap for free.

Do nothing, and accept the window. For some services a record admitted microseconds after the drain began is genuinely acceptable, because the work is idempotent and the caller retries. That is a legitimate engineering decision — but it must be a decision, stated in the code, not an accident of writing the check and the count as two statements.

What an interviewer is listening for

That you can produce the interleaving on demand, that you know the flag being atomic does not make the pair atomic, and that your fix names a mechanism that establishes mutual exclusion between "publishing closed" and "admitting". If you also mention that -race cannot find this, you have shown you understand the difference between a data race and a broken protocol.

## The window A drain built as "check a flag, then count the item" has two operations that are each atomic on their own — an atomic load and a `WaitGroup` increment — but nothing makes the *pair* atomic. Between them the goroutine can be preempted for an arbitrary length of time. Write out the interleaving explicitly: | step | submitting goroutine | shutdown goroutine | |---|---|---| | 1 | reads closing flag → false | | | 2 | (descheduled) | sets closing flag → true | | 3 | | calls Wait; counter is 0; returns | | 4 | | closes the output, exits | | 5 | increments counter, starts work | | At step 5 the record has been admitted by a service that already told the world it was drained. The work either never runs, or runs against a closed output, or is cut short by process exit. From the caller's perspective `Submit` returned `nil` and the record vanished, which is the single worst shutdown failure mode for something like an audit-log writer. There is a second problem hiding at step 5. `sync.WaitGroup` requires that an `Add` which raises the counter from zero *happen before* any `Wait` on that group; the reuse case is only safe when the previous `Wait` has fully returned. Step 5 races that rule directly. `sync` detects some of these and panics reporting WaitGroup misuse, so the bug can surface as a crash rather than as a quietly dropped record — which is, perversely, the luckier outcome. Note what the race detector will and will not do here. Both accesses to the flag are properly synchronised, so `-race` sees no data race on it. The defect is a *logical* race in the admission protocol, and only the `WaitGroup`'s own misuse check has any chance of catching it. You find this one by reasoning about the interleaving, not by running the detector. ## Making admission one step The requirement is precise: after the shutdown goroutine has published "closed", there must be no goroutine anywhere between the check and the increment. A reader-writer lock expresses exactly that. `Submit` takes the read lock, checks the flag, and — still holding it — calls `wg.Add(1)`, then releases. `Shutdown` takes the write lock, sets the flag, releases it, and only then calls `wg.Wait()`. The write lock cannot be acquired while any reader is inside its critical section, so acquiring it means every in-progress admission has completed its `Add`; and releasing it means every future admission will observe the flag as set. The counter is therefore at its final maximum before `Wait` is ever called, and can only fall from there. A read lock is the right shape rather than a plain mutex because admissions are concurrent with each other and only the single shutdown needs exclusivity. The critical section is tiny — a bool read and an integer increment — so the added contention is negligible next to the work being admitted. A plain `sync.Mutex` is perfectly acceptable too and is simpler to reason about; use it if admissions are not hot. Once the flag is under the lock, it no longer needs to be an atomic type at all. Keeping both an `atomic.Bool` *and* a mutex is a common half-fix: the atomic makes the individual read safe, which was never the problem. ## Variants you will see **One counter, with a closed sentinel.** Instead of a flag plus a `WaitGroup`, keep a single atomic counter and admit with a compare-and-swap loop that refuses whenever a "closed" bit is set. Admission and counting become literally one instruction, so the window cannot exist. Shutdown sets the bit and then waits for the count to reach zero, usually by having the last `Done` close a channel. It is more code and easier to get wrong, but it avoids a lock on a genuinely hot path. **A token channel.** Give the service a buffered channel of tokens sized to the concurrency limit, and admit only if a token can be taken *and* a `closing` channel is not yet closed, expressed as one `select`. Shutdown closes `closing` and then reclaims every token. Admission is again a single atomic decision, and the same structure gives you a concurrency cap for free. **Do nothing, and accept the window.** For some services a record admitted microseconds after the drain began is genuinely acceptable, because the work is idempotent and the caller retries. That is a legitimate engineering decision — but it must be a decision, stated in the code, not an accident of writing the check and the count as two statements. ## What an interviewer is listening for That you can produce the interleaving on demand, that you know the flag being atomic does not make the pair atomic, and that your fix names a mechanism that establishes mutual exclusion between "publishing closed" and "admitting". If you also mention that `-race` cannot find this, you have shown you understand the difference between a data race and a broken protocol.

Will the race detector find this defect?
No. Every access to the flag is properly synchronised, so there is no data race to report; the bug is a logical race in the admission protocol. At best the WaitGroup's own misuse check panics when the counter is raised off zero during a Wait. You find this by reasoning about the interleaving, not by running -race.
Why a sync.RWMutex rather than a plain sync.Mutex here?
Admissions are concurrent with each other and only the single shutdown needs exclusivity, so a read lock lets submissions proceed in parallel while still blocking the flag flip until every half-done admission has finished counting. A plain Mutex is correct too and simpler; prefer it unless the accept path is genuinely hot.
Is there a lock-free way to close the same window?
Yes. Keep one atomic counter with a reserved closed bit and admit with a compare-and-swap, so refusing and counting are a single operation. Or admit through a select that both takes a token from a buffered channel and checks a closing channel. Both are more code and easier to get subtly wrong than the lock.
Once the flag lives under the mutex, should it stay an atomic.Bool?
No. A plain bool guarded by the same lock is clearer and equally correct. Keeping both is a half-fix that suggests the author thought the unsafe part was reading the flag, when the unsafe part was the gap between reading it and counting the item.

saying these in an interview costs you the question

Making the flag atomic removes the race
The race detector would catch this
The window is too small to matter in practice
Add can be called freely while another goroutine waits
Rechecking the flag after the Add fixes it

What interleaving still drops a task when Go's Submit checks a closing flag and then calls wg.Add(1)?

§ answer

basics

solid answer

code

§ go deeper

▾level notes4 levels

▾deep dive~4 min

The window

Making admission one step

Variants you will see

What an interviewer is listening for

▾common follow-ups4 questions

▾red flags5 flags

The window

Making admission one step

Variants you will see

What an interviewer is listening for

answer

go deeper