flightcontrol: nil pointer dereference in (*progressState).run when a waiter is cancelled as the call finishes

クローズ 初心者向け
#7,253 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

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

関連するプルリクエストがすでにマージされています。

  • #7256 @crazy-max による — マージ済み

評価

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

調査の方向性

データ競合はutil/flightcontrol/flightcontrol.goにあります。303~308行目付近の(*progressState).runでは、lockを保持せずにwriterを閉じています。また、356行目付近のprogressState.closeでは、slices.Deleteがps.writersを変更しています。提案された修正を適用してください。lockの下でps.writersのスナップショットを取得し、その後、クリティカルセクションの外でスナップショットを閉じます。提供された再現テストを実行し、panicが発生しなくなったことを確認してください。

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

説明

Summary

dockerd crashes with a nil pointer dereference in util/flightcontrol.(*progressState).run. The cause is a data race between run and progressState.close.

When run reads io.EOF, it sets ps.done under ps.mu, releases the lock, and then ranges over ps.writers and calls Close on each one without the lock (flightcontrol.go:303-308 at v0.33.0). Meanwhile, a waiter whose context is cancelled while other waiters still hold the call reaches progressState.close. That function removes its writer under the lock with ps.writers = slices.Delete(ps.writers, i, i+1) (flightcontrol.go:356).

Since Go 1.22, slices.Delete zeroes the vacated tail element. The unlocked range in run still uses the old length, so it reads a nil rawProgressWriter and calls Close on it. addr=0x18 is the offset of the first method in the itab, and Close is that method.

The unlocked read itself was reported as a data race in #3923, which was closed without a code change. It became a crash with b5286f8dcb ("apply x/tools/modernize fixes", 2025-03-07). That commit replaced the append-based removal with slices.Delete. It first shipped in v0.21.0 (Docker 28.1.0), and the code is byte-identical in v0.33.0, v0.33.1 and master as of 2026-10-06.

Observed

Docker 29.8.0, vendoring BuildKit v0.33.0, as a rootless daemon with the containerd snapshotter, running several concurrent docker buildx bake sessions (docker driver) whose targets share identical stages:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x...]
goroutine ... [running]:
github.com/moby/moby/v2/vendor/github.com/moby/buildkit/util/flightcontrol.(*progressState).run(...)
        .../vendor/github.com/moby/buildkit/util/flightcontrol/flightcontrol.go:307 +0x201
created by github.com/moby/moby/v2/vendor/github.com/moby/buildkit/util/flightcontrol.newCall[...] in goroutine ...
        .../vendor/github.com/moby/buildkit/util/flightcontrol/flightcontrol.go:113 +0x245

The daemon exits, and every running container and build on it dies with it. On this host it happened once in about 5.5 days of continuous build load.

Reproducer

This is a standalone test against github.com/moby/buildkit at v0.33.0. It uses one shared call, one waiter that stays, and 32 waiters whose contexts are cancelled at about the moment the shared function returns:

package repro

import (
	"context"
	"math/rand"
	"sync"
	"testing"
	"time"

	"github.com/moby/buildkit/util/flightcontrol"
	"github.com/moby/buildkit/util/progress"
)

func TestProgressStateCloseRace(t *testing.T) {
	const iterations = 20000
	const cancelled = 32
	for i := 0; i < iterations; i++ {
		var g flightcontrol.Group[int]
		release := make(chan struct{})
		fn := func(ctx context.Context) (int, error) {
			<-release
			return 1, nil
		}
		var wg sync.WaitGroup
		_, keepCtx, keepClose := progress.NewContext(context.Background())
		wg.Add(1)
		go func() { defer wg.Done(); _, _ = g.Do(keepCtx, "k", fn) }()
		cancels := make([]context.CancelFunc, 0, cancelled)
		for j := 0; j < cancelled; j++ {
			_, pctx, _ := progress.NewContext(context.Background())
			ctx, cancel := context.WithCancel(pctx)
			cancels = append(cancels, cancel)
			wg.Add(1)
			go func() { defer wg.Done(); _, _ = g.Do(ctx, "k", fn) }()
		}
		time.Sleep(200 * time.Microsecond)
		go func() {
			time.Sleep(time.Duration(rand.Intn(40)) * time.Microsecond)
			close(release)
		}()
		for _, c := range cancels {
			c()
		}
		wg.Wait()
		keepClose(nil)
	}
}

Results with Go 1.26:

  • v0.33.0 panics within a fraction of a second with the same trace (flightcontrol.go:307 and :113, addr=0x18). Under -race it reports a DATA RACE between slices.Delete in close and Close in run.
  • v0.21.0 panicked in 3 of 3 runs.
  • v0.20.2, which uses append-based removal, passed in 3 of 3 runs.
Suggested fix

Snapshot the writers while holding the lock, then close the snapshot:

if errors.Is(err, io.EOF) {
	ps.mu.Lock()
	ps.done = true
	writers := slices.Clone(ps.writers)
	ps.mu.Unlock()
	for _, w := range writers {
		w.Close()
	}
}

With this change, v0.33.0 passed the reproducer in 3 of 3 runs of 20,000 iterations. Closing the writers while holding the lock would also work, but cloning keeps Close outside the critical section.

主要言語
Go
スター
10.4k
フォーク
1.6k
平均マージ
3日 6時間
マージ済み PR(30日)
75

環境構築

はじめの一歩

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

moby/buildkit のほかの issue

moby/buildkit の issue をすべて見る

似ている issue

Go の issue をもっと見る

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

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