Hacktoberfest 2026：メンテナが10月に向けて印を付けた、オープンで初心者向けの issue。 Hacktoberfest の issue を見る

bug(orchestrator): Cleanup.Add has TOCTOU race — cleanup functions silently dropped after Run()

オープン
  • 何か問題がありますか？
#3,557 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
3/5
見積もり時間
1〜2日
初心者へのやさしさ
72/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
静か
技術スタック
go
領域
infrastructure

調査の方向性

packages/orchestrator/pkg/sandbox/cleanup.go の Cleanup.Add、Cleanup.AddPriority、Cleanup.run を確認し、lock と hasRun の順序に注目してください。変更によって TOCTOU ウィンドウが解消され、Run() と同時に登録されたクリーンアップ関数が暗黙のうちに破棄されなくなったことを確認し、その後、race detector を使ってパッケージの既存テストを実行してください。

索引モデルが issue の本文から書いたものです。

説明

Summary

Cleanup.Add and Cleanup.AddPriority have a TOCTOU (time-of-check/time-of-use) race against Cleanup.run that silently discards cleanup functions without executing them, causing permanent resource leaks.

Root Cause

In packages/orchestrator/pkg/sandbox/cleanup.go:

run() sets hasRun before acquiring the lock (line 80 vs 82):

func (c *Cleanup) run(ctx context.Context) {
    c.hasRun.Store(true)   // ← written outside the lock
    c.mu.Lock()
    defer c.mu.Unlock()
    // ... drains c.cleanup and c.priorityCleanup
}

Add() reads hasRun before acquiring the lock (line 40 vs 49):

func (c *Cleanup) Add(ctx context.Context, f func(ctx context.Context) error) {
    if c.hasRun.Load() == true {   // ← read outside the lock
        // run f immediately
        return
    }
    c.mu.Lock()
    defer c.mu.Unlock()
    c.cleanup = append(c.cleanup, f)   // ← appended after run() may have drained
}

Race Window

Goroutine A (Add):    hasRun.Load() == false  →  [not yet locked]
Goroutine B (run):    hasRun.Store(true)  →  Lock()  →  drain all cleanup  →  Unlock()
Goroutine A (Add):    Lock()  →  append(f)   ← f is now in a drained slice, never executed

sync.Once in Run() prevents run() from being called a second time, so f is permanently lost — no log, no error, no signal.

Impact

The Cleanup type is the central resource-release mechanism for Firecracker sandbox lifecycle. The package has 28 call sites across sandbox startup (sandbox.go, resume.go, reboot.go, etc.) registering operations such as:

  • Closing overlay filesystems
  • Releasing network slots
  • Removing Firecracker and UFFD socket files
  • Stopping the Firecracker process

Any one of these silently dropped during a concurrent error-path teardown leaves a leaked resource on the host until the orchestrator restarts.

Fix

Move hasRun.Store(true) inside the lock in run(), and add a double-check in Add() / AddPriority() after acquiring mu:

func (c *Cleanup) Add(ctx context.Context, f func(ctx context.Context) error) {
    // Optimistic fast path.
    if c.hasRun.Load() {
        err := f(context.WithoutCancel(ctx))
        if err != nil {
            logger.L().Error(ctx, "failed to run function after cleanup has run", zap.Error(err))
        }
        return
    }

    c.mu.Lock()
    // Double-check: run() may have completed between the Load above and here.
    if c.hasRun.Load() {
        c.mu.Unlock()
        err := f(context.WithoutCancel(ctx))
        if err != nil {
            logger.L().Error(ctx, "failed to run function after cleanup has run", zap.Error(err))
        }
        return
    }
    c.cleanup = append(c.cleanup, f)
    c.mu.Unlock()
}

func (c *Cleanup) run(ctx context.Context) {
    c.mu.Lock()
    defer c.mu.Unlock()

    c.hasRun.Store(true)  // now set under the lock — closes the race window

    // ... rest unchanged
}

The same double-check applies to AddPriority.

主要言語
Go
スター
1.6k
フォーク
438
30日以内にマージされた PR はありません

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

e2b-dev/runtime のほかの issue

e2b-dev/runtime の issue をすべて見る

似ている issue

Go の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。