Debug this channel fan-in merge: range merge(...) deadlocks. Find both bugs.
This merge fans several channels into one out channel, with a goroutine meant to close out once all producers finish. Ranging over merge(...) deadlocks. There are two bugs.
func merge(chs ...chan int) chan int {
out := make(chan int)
wg := &sync.WaitGroup{}
for _, c := range chs {
wg.Add(1)
go func() {
defer wg.Done()
for val := range c {
out <- val
}
}()
}
go func() {
wg.Wait()
close(out)
}
return out
}
func main() {
ch1 := startProducerA()
ch2 := startProducerB()
for el := range merge(ch1, ch2) {
println(el)
}
}
Find and fix the bug.
Two bugs. The closing goroutine is written go func(){ wg.Wait(); close(out) } with no trailing (), so it is declared but never launched — out never closes and range merge(...) deadlocks. Second, the worker closure captures the loop variable c, so pre-1.22 every goroutine ranges over the same last channel. Fix: call the closer with (), and pass c as an argument (or rebind it).
- ✗Reading
go func(){...}as a launched goroutine when the missing()makes it just a declared, never-called value - ✗Assuming pre-1.22 loop variables are per-iteration, so the captured
cis the last channel for every worker - ✗Blaming the deadlock on a
close/send race instead of onoutnever being closed
- →Why does Go 1.22's per-iteration loop variable remove the need to pass
cas an argument? - →How would
go vetor the race detector help you catch the loop-variable capture before runtime?
Buggy code
func merge(chs ...chan int) chan int {
out := make(chan int)
wg := &sync.WaitGroup{}
for _, c := range chs {
wg.Add(1)
go func() {
defer wg.Done()
for val := range c { // BUG 2: pre-1.22 captures loop variable c
out <- val
}
}()
}
go func() {
wg.Wait()
close(out)
} // BUG 1: no trailing () — goroutine declared, never launched
return out
}
func main() {
ch1 := startProducerA()
ch2 := startProducerB()
for el := range merge(ch1, ch2) { // deadlock: out is never closed
println(el)
}
}
What is wrong
Bug 1 — the closer never runs. go func(){ wg.Wait(); close(out) } without () is a function literal handed to go, but the function is never called. The closing goroutine never starts, out is never closed, and range merge(...) in main blocks forever once the values run out. It is a deadlock, not a panic.
Bug 2 — loop-variable capture. Before Go 1.22 the variable c is shared across the whole loop. Every worker goroutine closes over the same c, which by the time they run holds the last channel. Some producers are never drained; the result is non-deterministic.
The fix
func merge(chs ...chan int) chan int {
out := make(chan int)
wg := &sync.WaitGroup{}
for _, c := range chs {
wg.Add(1)
go func(c chan int) { // pass c as an argument (pre-1.22)
defer wg.Done()
for val := range c {
out <- val
}
}(c)
}
go func() {
wg.Wait()
close(out)
}() // launch the closing goroutine
return out
}
On Go 1.22+ the loop variable is per-iteration, so passing c as an argument is no longer required — but the () on the closing goroutine is mandatory in every version.