Goのselect defaultでキャンセル不能になるループをどうレビューするか
Goのselect defaultでキャンセル不能になるループをどうレビューするか
Goの select に default を入れると、チャネルが準備できていないときでも処理を進められる。
非ブロッキング処理には便利だが、ループの中で安易に使うとキャンセル不能なbusy loopになりやすい。
レビューで危ないのは、次のような実装だ。
defaultがあるため、ctx.Done()を待たずにループが回り続ける- 受信できない時間帯にCPUを消費し続ける
time.Sleepで無理に抑えていて、停止責務が読めない- goroutineの終了条件が呼び出し元から制御できない
この記事では select default の是非ではなく、
ループがいつ止まり、待機中に何を消費するのかをレビューで見る観点を整理する。
まず止めたい実装
func StartWorker(ctx context.Context, jobs <-chan Job) {
go func() {
for {
select {
case job := <-jobs:
process(job)
default:
refreshMetrics()
}
@Reviewer`default` によりjobsが空の間もループが止まらず、CPUを消費し続けます。待機したい処理なのか、周期実行したい処理なのかを分けてください。 }
}()
}このコードは、ジョブが来たら処理し、来ていないときはメトリクス更新するように見える。
しかし実際には、ジョブがない間 refreshMetrics() を全速力で呼び続ける。
さらに ctx を受け取っているのに、終了条件として使われていない。
レビューでは、これはかなり早く止めたい構造である。
なぜ危ないのか
select に default があると、どのcaseも実行できないときに即座にdefaultへ進む。
ループの中では、これは「待たない」という意味になる。
その結果、次の問題が起きやすい。
- CPU使用率が上がる
- ログやメトリクスが過剰に出る
ctxキャンセル後もgoroutineが残る- テストでは短時間すぎて問題が見えない
- 本番でアイドル時だけ負荷が出る
レビューで見るべきなのは、default があるかどうかだけではない。
そのループがブロックすべき場面で本当に待てているかである。
レビューで見たい3つの判断線
1. default が「待機しない理由」を持っているか
非ブロッキングである必要が説明できるなら default は有効だ。
しかし単に「詰まらないように入れた」なら危険である。
@Reviewer: この `default` によりチャネル受信待ちが発生しません。非ブロッキングである必要が仕様として説明できない場合は、`default` を外して待機する構造にしてください。2. ctx.Done() が終了条件として機能しているか
ctx を引数で受け取っていても、select内に入っていなければ停止できない。
また、default が常に実行される構造では、キャンセルを見落としやすい。
@Reviewer: `ctx` を受け取っていますが、ループ終了条件に使われていません。呼び出し元がworkerを停止できるよう、`case <-ctx.Done()` をselectに含めてください。3. 周期処理なら ticker で間隔を表現しているか
「ジョブがないときに時々何かする」なら、default ではなく time.Ticker で周期を明示するほうがよい。
@Reviewer: `refreshMetrics()` はアイドル時の都度実行ではなく周期実行に見えます。`default` ではなく `time.Ticker` を使い、実行間隔と停止責務をコードで表現してください。改善例:待機と周期実行を分ける
ジョブ処理とメトリクス更新を同じループで扱うなら、ticker と ctx.Done() を明示する。
func StartWorker(ctx context.Context, jobs <-chan Job) {
go func() {
ticker := time.NewTicker(30 * time.Second)
defer ticker.Stop()
for {
select {
case <-ctx.Done():
return
case job, ok := <-jobs:
if !ok {
return
}
process(job)
case <-ticker.C:
refreshMetrics()
}
}
}()
}この構造なら、レビューアーは次を確認しやすい。
ctxキャンセルでgoroutineが止まるjobsclose時にも終了する- メトリクス更新の周期が明示されている
- tickerの停止責務がある
- チャネルが空でもCPUを使い続けない
default が許容されるケース
select default が常に悪いわけではない。
レビューで通しやすいのは、次のように「試すだけ」の処理だ。
func TryNotify(ch chan<- Event, event Event) bool {
select {
case ch <- event:
return true
default:
return false
}
}この関数はループしていない。
送信できなければ false を返し、呼び出し側が次の判断をする。
@Reviewer: この `default` は非ブロッキング送信の成否をboolで返しており、ループ内でCPUを消費する構造ではありません。呼び出し側が失敗時の扱いを持つなら妥当です。問題は、default が無限ループと組み合わさったときだ。
その場合は、待機しない理由と停止条件を必ず確認したい。
レビュー観点チェックリスト
defaultがループ内でbusy loopを作っていないか- 非ブロッキングである必要が仕様として説明できるか
ctx.Done()やチャネルcloseでgoroutineが終了するか- 周期処理を
defaultで代用していないか time.Tickerを使う場合にStop()されているか- テストで停止確認やキャンセル確認ができる構造か
まとめ
select default は「待たない」ための構文である。
その性質を理解せずにループへ入れると、アイドル時にCPUを消費し、キャンセルできないgoroutineを作りやすい。
レビューでは、default の有無だけでなく、
待つべきところで待てているか、止めるべきときに止められるかを確認したい。