Goのselect defaultでキャンセル不能になるループをどうレビューするか

Goの selectdefault を入れると、チャネルが準備できていないときでも処理を進められる。
非ブロッキング処理には便利だが、ループの中で安易に使うとキャンセル不能なbusy loopになりやすい

レビューで危ないのは、次のような実装だ。

  • default があるため、ctx.Done() を待たずにループが回り続ける
  • 受信できない時間帯にCPUを消費し続ける
  • time.Sleep で無理に抑えていて、停止責務が読めない
  • goroutineの終了条件が呼び出し元から制御できない

この記事では select default の是非ではなく、
ループがいつ止まり、待機中に何を消費するのかをレビューで見る観点を整理する。

まず止めたい実装

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 を受け取っているのに、終了条件として使われていない。
レビューでは、これはかなり早く止めたい構造である。

なぜ危ないのか

selectdefault があると、どのcaseも実行できないときに即座にdefaultへ進む。
ループの中では、これは「待たない」という意味になる。

その結果、次の問題が起きやすい。

  • CPU使用率が上がる
  • ログやメトリクスが過剰に出る
  • ctx キャンセル後もgoroutineが残る
  • テストでは短時間すぎて問題が見えない
  • 本番でアイドル時だけ負荷が出る

レビューで見るべきなのは、default があるかどうかだけではない。
そのループがブロックすべき場面で本当に待てているかである。

レビューで見たい3つの判断線

1. default が「待機しない理由」を持っているか

非ブロッキングである必要が説明できるなら default は有効だ。
しかし単に「詰まらないように入れた」なら危険である。

Comment
@Reviewer: この `default` によりチャネル受信待ちが発生しません。非ブロッキングである必要が仕様として説明できない場合は、`default` を外して待機する構造にしてください。

2. ctx.Done() が終了条件として機能しているか

ctx を引数で受け取っていても、select内に入っていなければ停止できない。
また、default が常に実行される構造では、キャンセルを見落としやすい。

Comment
@Reviewer: `ctx` を受け取っていますが、ループ終了条件に使われていません。呼び出し元がworkerを停止できるよう、`case <-ctx.Done()` をselectに含めてください。

3. 周期処理なら ticker で間隔を表現しているか

「ジョブがないときに時々何かする」なら、default ではなく time.Ticker で周期を明示するほうがよい。

Comment
@Reviewer: `refreshMetrics()` はアイドル時の都度実行ではなく周期実行に見えます。`default` ではなく `time.Ticker` を使い、実行間隔と停止責務をコードで表現してください。

改善例:待機と周期実行を分ける

ジョブ処理とメトリクス更新を同じループで扱うなら、tickerctx.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が止まる
  • jobs close時にも終了する
  • メトリクス更新の周期が明示されている
  • tickerの停止責務がある
  • チャネルが空でもCPUを使い続けない

default が許容されるケース

select default が常に悪いわけではない。
レビューで通しやすいのは、次のように「試すだけ」の処理だ。

送れたら送るだけの非ブロッキング通知
func TryNotify(ch chan<- Event, event Event) bool {
    select {
    case ch <- event:
        return true
    default:
        return false
    }
}

この関数はループしていない。
送信できなければ false を返し、呼び出し側が次の判断をする。

Comment
@Reviewer: この `default` は非ブロッキング送信の成否をboolで返しており、ループ内でCPUを消費する構造ではありません。呼び出し側が失敗時の扱いを持つなら妥当です。

問題は、default が無限ループと組み合わさったときだ。
その場合は、待機しない理由と停止条件を必ず確認したい。

レビュー観点チェックリスト

select defaultレビューの確認項目
  • default がループ内でbusy loopを作っていないか
  • 非ブロッキングである必要が仕様として説明できるか
  • ctx.Done() やチャネルcloseでgoroutineが終了するか
  • 周期処理を default で代用していないか
  • time.Ticker を使う場合に Stop() されているか
  • テストで停止確認やキャンセル確認ができる構造か

まとめ

select default は「待たない」ための構文である。
その性質を理解せずにループへ入れると、アイドル時にCPUを消費し、キャンセルできないgoroutineを作りやすい。

レビューでは、default の有無だけでなく、
待つべきところで待てているか、止めるべきときに止められるかを確認したい。