Skip to content

Fix WaitGroup race between goAsyncWithSem and Close in DualWriter#355

Merged
bootjp merged 2 commits into
feature/redis-proxyfrom
copilot/sub-pr-351
Mar 19, 2026
Merged

Fix WaitGroup race between goAsyncWithSem and Close in DualWriter#355
bootjp merged 2 commits into
feature/redis-proxyfrom
copilot/sub-pr-351

Conversation

Copilot AI commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

DualWriter.Close() could panic with "WaitGroup misuse: Add called concurrently with Wait" because the atomic closing flag check and wg.Add(1) in goAsyncWithSem were not atomic — Close() could start wg.Wait() in the gap between them.

Changes

  • proxy/dualwrite.go: Replace closing int32 (atomic) with a sync.Mutex-protected closed bool. Hold the mutex across both the closed-flag check and wg.Add(1) in goAsyncWithSem; Close() sets closed = true under the same mutex before calling wg.Wait().
// goAsyncWithSem — wg.Add(1) now happens atomically with the closed check
d.mu.Lock()
if d.closed {
    d.mu.Unlock()
    return
}
select {
case sem <- struct{}{}:
    d.wg.Add(1)   // safe: Close cannot reach wg.Wait() until we release mu
    d.mu.Unlock()
    go func() { ... }()
default:
    d.mu.Unlock()
    // drop
}

// Close — sets closed before Wait, under the same mutex
d.mu.Lock()
d.closed = true
d.mu.Unlock()
d.wg.Wait()
  • proxy/proxy_test.go: Add TestDualWriter_Close_NoWaitGroupRace — 50 goroutines hammering goAsync concurrently with Close(), run under -race.

📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

…Wait race in DualWriter

Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
Copilot AI changed the title [WIP] [WIP] Address feedback on dual-write Redis proxy implementation Fix WaitGroup race between goAsyncWithSem and Close in DualWriter Mar 19, 2026
Copilot AI requested a review from bootjp March 19, 2026 17:55
@bootjp

bootjp commented Mar 19, 2026

Copy link
Copy Markdown
Owner

@copilot どうして wait group を使ってるんですかmutexだけではダメですか?

Copilot AI commented Mar 19, 2026

Copy link
Copy Markdown
Contributor Author

@copilot どうして wait group を使ってるんですかmutexだけではダメですか?

ミューテックスだけでは足りない理由は、「すでに起動中のゴルーチンが終わるまで待つ」機能が必要だからです。

mu は「closed フラグのチェック」と「wg.Add(1)」を不可分にする役割で、競合(race)を防ぎます。しかし、一度 go func() で起動したゴルーチンが fn()(例: セカンダリへの書き込み)を実行中の場合、Close() はその完了を待たなければなりません。sync.Mutex はこの「N 個の処理が完了するまでブロックする」セマンティクスを持ちません。

代替として sync.RWMutex を使うパターンもあります(ゴルーチンが RLock を保持し続け、Close()Lock() を取得して全 RLock 解放を待つ)。ただしこの場合、Close()Lock() を要求した後に新たな RLock() 呼び出しがブロック(すぐに drop ではなく待機)してしまうため、シャットダウン中の新規ワークを即座に捨てる動作が失われます。

WaitGroup は「N 個のゴルーチンの完了を待つ」という Go の慣用パターンであり、ここでは mutex との組み合わせが最もシンプルかつ正確な実装です。

@bootjp bootjp marked this pull request as ready for review March 19, 2026 18:08
Copilot AI requested a review from bootjp March 19, 2026 18:09
@bootjp bootjp merged commit f775a3c into feature/redis-proxy Mar 19, 2026
5 checks passed
@bootjp bootjp deleted the copilot/sub-pr-351 branch March 19, 2026 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants