1
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

`async void` で本番在庫が溶けた話 ── .NET の非同期で踏んだ地雷と、設計をどう直したか

1
Posted at

はじめに

金曜の夜 22 時、Slack の通知音で飛び起きました。

「在庫数がマイナスになってる商品が 80 件くらいあります。どうなってますか?」

ECサイトのバックエンドを担当している私のところに、運用チームからの一報。在庫が「マイナス」というのは、要するに売れていない数まで「売れた」ことになっている状態です。返金対応、在庫の手動補正、原因調査……結局その週末はほぼ潰れました。

調べていくと、犯人は数日前に私が何気なく書いた たった 1 行の async void でした。

この記事は、その障害の原因を .NET の非同期処理の仕組みから掘り下げて、「なぜ起きたのか」「どう直したのか」「これからどう書けばいいのか」をまとめたものです。async/await を普段なんとなく使っている方が、同じ地雷を踏まないための備忘録として書きました。

環境は .NET 8 / C# 12、ワーカーサービス(BackgroundService)構成です。


何が起きていたのか

うちの在庫同期は、外部倉庫システムから届く「在庫変動イベント」を受け取って、自社DBの在庫数を更新する、というシンプルな処理です。イベントが来るたびに、こんな感じのハンドラが呼ばれます。

ある日、私は「在庫を更新したついでに、関連する検索インデックスも非同期で更新しておこう」と思い立ち、軽い気持ちでこう書きました。

public class StockEventHandler
{
    private readonly IStockRepository _repository;
    private readonly ISearchIndexer _indexer;

    public void OnStockChanged(StockChangedEvent e)
    {
        // 在庫を更新する
        _repository.UpdateStock(e.ProductId, e.NewQuantity);

        // ついでに検索インデックスも更新(非同期で投げっぱなし)
        UpdateIndexAsync(e.ProductId);   // ← これが地雷
    }

    private async void UpdateIndexAsync(int productId)   // ← async void
    {
        var product = await _repository.GetProductAsync(productId);
        await _indexer.IndexAsync(product);
    }
}

一見、問題なさそうに見えませんか? 私もそう思っていました。レビューも通りました。テストもグリーンでした。

でも本番では、インデックス更新が失敗したとき、在庫更新まで巻き添えにしておかしくなるという、一見すると無関係な壊れ方をしたのです。

ここから先は、なぜこんなことが起きたのかの謎解きです。


地雷その1: async void は例外を「闇」に葬る

最初の、そして最大の問題は async void です。

async メソッドの戻り値には TaskTask<T>、そして void の 3 種類があります。このうち void特別扱いで、呼び出し側が await できません。await できないということは、中で起きた例外を呼び出し側が受け取れないということです。

通常の async Task であれば、例外は返ってきた Task の中に格納され、await した瞬間に再スローされます。ところが async void の場合、待ち受ける Task が存在しないため、例外は行き場を失い、その時点の SynchronizationContext(同期コンテキスト)へ直接投げつけられます。

これを図にするとこうなります。

ワーカーサービス構成では SynchronizationContextnull なので、この例外は最終的に スレッドプールのスレッド上で未処理例外になります。.NET では未処理例外はデフォルトでプロセスを落とします。

つまり私のコードは、「検索インデックスの更新に失敗すると、在庫同期のワーカープロセスごと突然死する」という時限爆弾だったわけです。

実際のクラッシュ時のログを再現するとこんな感じでした。

Unhandled exception. System.Net.Http.HttpRequestException:
  No connection could be made because the target machine actively refused it.
   at SearchIndexer.IndexAsync(Product product)
   at StockEventHandler.UpdateIndexAsync(Int32 productId)
--- End of stack trace ---
Application is shutting down...

検索エンジン側が一時的に過負荷で接続を拒否しただけ。本来なら「インデックス更新だけ後回しにすればいい」軽微な事象です。それが在庫ワーカーを巻き込んで落としていました。


地雷その2: プロセス再起動と「途中まで処理されたイベント」

ワーカーが落ちると、当然オーケストレーター(うちは Kubernetes)が再起動してくれます。ここで2つ目の問題が顔を出しました。

在庫変動イベントはメッセージキューから取得していて、「処理が終わったら ack(確認応答)を返す」設計でした。ところが私の OnStockChanged は、UpdateIndexAsyncawait せずに投げっぱなしにしていたため、インデックス更新が終わる前に ack が返ってしまうことがありました。

タイミングが悪いと、こうなります。

問題の本質は「await していないのに、処理が完了したと宣言してしまった」点です。投げっぱなしのタスクは、メインの処理フローから見ると存在しないも同然。完了も失敗も観測されません。

在庫がマイナスになった直接の引き金は、この「順序保証の崩壊」と、後述する再試行ロジックの合わせ技でした。クラッシュ → 再起動 → 別ワーカーが古いイベントを拾う、という流れの中で、同じ商品の在庫更新が二重・逆順に適用され、結果として整合性が壊れたのです。


地雷その3: 慌てて入れた .Result でデッドロック

障害対応の最中、私は「とりあえずインデックス更新を確実に待たせよう」と考えて、こんな突貫修正を入れかけました。これは絶対にやってはいけない例として残しておきます。

public void OnStockChanged(StockChangedEvent e)
{
    _repository.UpdateStock(e.ProductId, e.NewQuantity);

    // 同期的に待たせれば安全……だと思った(大間違い)
    UpdateIndexAsync(e.ProductId).Result;   // ← デッドロック製造機
}

private async Task UpdateIndexAsync(int productId)
{
    var product = await _repository.GetProductAsync(productId);
    await _indexer.IndexAsync(product);
}

.Result(や .Wait())は、非同期処理を同期的に「待つ」ためのものですが、SynchronizationContext がある環境では デッドロックを引き起こします。

仕組みはこうです。await の後の処理は、デフォルトで「元のコンテキストに戻って続きを実行しよう」とします。ところが .Result で待っている呼び出し元が、まさにそのコンテキストを 占有してブロックしています。続きを実行したいスレッドは「コンテキストが空くまで待つ」、コンテキストを握っているスレッドは「続きが終わるまで待つ」。お互いがお互いを待ち続ける、典型的なデッドロックです。

ステージング環境でこれを試したら見事に固まり、肝が冷えました。「非同期を同期で待つ」のは、最も避けるべき選択肢です。


どう直したのか

ここからが本題、設計の立て直しです。ポイントは3つでした。

1. async voidasync Task にして、ちゃんと await する

そもそも投げっぱなしが諸悪の根源でした。ハンドラ全体を async Task にして、インデックス更新まで含めて1つの処理として待ち切る形に変えます。

public class StockEventHandler
{
    private readonly IStockRepository _repository;
    private readonly ISearchIndexer _indexer;
    private readonly ILogger<StockEventHandler> _logger;

    // void → Task に変更し、呼び出し元が await できるようにする
    public async Task OnStockChangedAsync(
        StockChangedEvent e, CancellationToken ct)
    {
        await _repository.UpdateStockAsync(e.ProductId, e.NewQuantity, ct);

        // インデックス更新も同じフローの中で await する
        await UpdateIndexAsync(e.ProductId, ct);
    }

    private async Task UpdateIndexAsync(int productId, CancellationToken ct)
    {
        var product = await _repository.GetProductAsync(productId, ct);
        await _indexer.IndexAsync(product, ct);
    }
}

これで例外は呼び出し元まで正しく伝播し、try/catch で受け止められるようになりました。CancellationToken も最後まで通して、シャットダウン時にきれいに止まるようにしています。

2. 「失敗しても本処理を止めたくない」処理は明示的に分離する

とはいえ、本来の要件は「在庫更新は絶対に守りたい、インデックス更新はベストエフォートでいい」でした。この優先度の違いを、コードではっきり表現します。投げっぱなしにするのではなく、「失敗を許容することを意図して書いた」と分かる形にするのが肝心です。

public async Task OnStockChangedAsync(
    StockChangedEvent e, CancellationToken ct)
{
    // 在庫更新は絶対に守る。失敗したら例外を上げて再試行に回す
    await _repository.UpdateStockAsync(e.ProductId, e.NewQuantity, ct);

    // インデックス更新はベストエフォート。
    // 失敗してもログだけ残して本処理は止めない、と「意図的に」書く
    try
    {
        await UpdateIndexAsync(e.ProductId, ct);
    }
    catch (Exception ex)
    {
        _logger.LogWarning(ex,
            "検索インデックスの更新に失敗しました (ProductId={ProductId})。" +
            "在庫更新は完了済みのため処理は継続します。", e.ProductId);

        // 後で拾い直すために、失敗分を再インデックスキューに積む
        await _indexer.EnqueueRetryAsync(e.ProductId, ct);
    }
}

ポイントは、catch の中で 何もしないのではなく、ログを残し、リトライキューに積んでいることです。「握りつぶし」と「意図的なベストエフォート」は、見た目が似ていてもまったくの別物です。後者は「失敗を観測し、回復の道を用意している」状態を指します。

3. 同期キューと await の順序を直して ack の意味を正す

最後に、メッセージの ack を「本当に処理が完了してから返す」ように直しました。await で処理全体を待ち切ってから ack するだけで、投げっぱなしによる順序崩壊は解消します。

修正前後の流れを並べるとこうです。

地味な変更ですが、「ack=完了の宣言」という契約を守るだけで、システムの信頼性は劇的に上がりました。


学んだこと・気づいたこと

今回の障害から得た教訓を、自分への戒めとしてまとめます。

async void は、イベントハンドラ以外では原則使わない。 UI のボタンクリックハンドラなど、シグネチャ上どうしても void でなければならない場面以外で async void を見かけたら、それは async Task の書き間違いだと思ってほぼ間違いありません。私のチームでは、後述の Analyzer で機械的に検出するようにしました。

「投げっぱなし(fire-and-forget)」は、思っているより危険。 タスクを await せずに放流すると、その成功も失敗も誰も観測しません。本当に投げっぱなしにしたい正当な理由があるなら、最低でも例外を握って必ずログに残すラッパー(_ = SafeFireAndForget(...) のような明示的な仕組み)を通すべきです。何も考えず呼び出すのとは天と地の差があります。

「非同期を同期で待つ」(.Result / .Wait())は最終手段ですらない。 デッドロックの温床です。上から下まで async で貫く(async all the way)のが基本で、どうしても同期境界が必要なら設計を見直すサインだと捉えるべきでした。

例外の握りつぶしと、意図的なベストエフォートは別物。 catch で黙って握りつぶすのは事故。「失敗を許容する」なら、ログ・メトリクス・リトライのいずれかで必ず失敗を観測可能にする。これが両者を分ける一線です。


おまけ: 二度と踏まないための仕組み化

最後に、人間の注意力に頼らないための予防策を2つ。

ひとつは Roslyn Analyzer による静的検出です。Microsoft.VisualStudio.Threading.Analyzers を入れると、async voidVSTHRD100)や .Result / .Wait() の同期待ち(VSTHRD002)をビルド時に警告してくれます。.editorconfig でこれらを警告ではなくエラーに昇格させておけば、そもそもマージできなくなります。

# .editorconfig
[*.cs]
# async void を禁止(イベントハンドラ以外)
dotnet_diagnostic.VSTHRD100.severity = error
# 非同期処理の同期待ち(.Result / .Wait())を禁止
dotnet_diagnostic.VSTHRD002.severity = error

もうひとつは 未観測タスク例外の監視です。投げっぱなしタスクの例外がもし発生しても、せめて気づけるように TaskScheduler.UnobservedTaskException を購読して、メトリクスに飛ばすようにしました。

TaskScheduler.UnobservedTaskException += (sender, args) =>
{
    _logger.LogError(args.Exception,
        "観測されなかったタスク例外を検出しました。" +
        "fire-and-forget の取りこぼしの可能性があります。");
    args.SetObserved();   // プロセス巻き込みを防ぐ
};

これは「最後の砦」であって、本来は発火しないのが正しい状態です。ですが、もし鳴ったら「どこかにまた投げっぱなしが紛れ込んだ」というアラートになります。


おわりに

たった 1 行の async void が、週末ひとつと、80 件分の在庫補正と、それなりの胃の痛みを生みました。けれど、おかげで非同期処理の仕組みを腰を据えて理解し直すきっかけにもなりました。

async/await は、書くのは簡単なのに、ちゃんと理解して使うのは難しい機能だと改めて感じます。この記事が、過去の私と同じように「なんとなく」非同期を書いている誰かの、週末を守れたら嬉しいです。

最後まで読んでいただきありがとうございました。同じような失敗談やツッコミがあれば、ぜひコメントで教えてください。

1
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
1
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?