Skip to content

[carry] history: fix panic when listing history#6614

Merged
tonistiigi merged 2 commits intomoby:masterfrom
tonistiigi:fix-history-panic2
Mar 25, 2026
Merged

[carry] history: fix panic when listing history#6614
tonistiigi merged 2 commits intomoby:masterfrom
tonistiigi:fix-history-panic2

Conversation

@tonistiigi
Copy link
Member

@tonistiigi tonistiigi commented Mar 25, 2026

carry of #6598 with extra commit

Seemed that #6598 missed the actual cause of the nil events and still returned nils by just ignoring them inside the sorter to avoid panic.

Also fix linter issues.

I observed following panics couple of times while running
'docker buildx history ls'. This commit should fix it together with test
which allows reproducing the panic.

Stacktrace:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x5571e75c90ae]
goroutine 16989985 [running]:
github.com/moby/buildkit/solver/llbsolver.filterHistoryEvents.func1(0xc015db4740?, 0x26?)
        github.com/moby/buildkit@v0.27.1/solver/llbsolver/history.go:1074 +0xe
slices.partitionCmpFunc[...]({0xc0014bb400?, 0x35, 0x40}, 0x0?, 0x7f13d47d6130, 0x5571e894e008?, 0x5571e894e008?)
        slices/zsortanyfunc.go:154 +0x27b
slices.pdqsortCmpFunc[...]({0xc0014bb400, 0x35, 0x40}, 0x5571e5660de7, 0xc00eeb1c98?, 0xc0011757a8?, 0x5571e894e008?)
        slices/zsortanyfunc.go:114 +0x2fe
slices.SortFunc[...](...)
        slices/sort.go:32
github.com/moby/buildkit/solver/llbsolver.filterHistoryEvents({0xc0014bb400, 0x35, 0x40}, {0x0?, 0x5571e7b577f2?, 0xc00442b410?}, 0x32)
        github.com/moby/buildkit@v0.27.1/solver/llbsolver/history.go:1073 +0x134
github.com/moby/buildkit/solver/llbsolver.(*HistoryQueue).Listen(0xc001b979e0, {0x5571e8999588, 0xc00d0ae930}, 0xc002e37e30, 0xc001175a28)
        github.com/moby/buildkit@v0.27.1/solver/llbsolver/history.go:1011 +0x80e
github.com/moby/buildkit/control.(*Controller).ListenBuildHistory(0xc0004c1500, 0xc002e37e30, {0x5571e89a8e40, 0xc00442b3e0})
        github.com/moby/buildkit@v0.27.1/control/control.go:311 +0xaf
github.com/moby/buildkit/api/services/control._Control_ListenBuildHistory_Handler({0x5571e8825da0, 0xc0004c1500}, {0x5571e89a4798, 0xc003c6c300})
        github.com/moby/buildkit@v0.27.1/api/services/control/control_grpc.pb.go:351 +0x110
github.com/moby/buildkit/util/grpcerrors.StreamServerInterceptor({0x5571e8825da0, 0xc0004c1500}, {0x5571e89a4798, 0xc003c6c300}, 0x5571e81b5140?, 0x5571e894bfc8)
        github.com/moby/buildkit@v0.27.1/util/grpcerrors/intercept.go:33 +0x5f
google.golang.org/grpc.(*Server).processStreamingRPC(0xc00189d448, {0x5571e8999588, 0xc00d0ae7b0}, 0xc002533a00, 0xc001362180, 0x5571ea24c8c0, 0x0)
        google.golang.org/grpc@v1.78.0/server.go:1721 +0x1151
google.golang.org/grpc.(*Server).handleStream(0xc00189d448, {0x5571e899b0d0, 0xc001f3ac30}, 0xc002533a00)
        google.golang.org/grpc@v1.78.0/server.go:1836 +0xd85
google.golang.org/grpc.(*Server).serveStreams.func2.1()
        google.golang.org/grpc@v1.78.0/server.go:1063 +0x7f
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 16989982
        google.golang.org/grpc@v1.78.0/server.go:1074 +0x11d

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Comment on lines +28 to +35
events := make([]*controlapi.BuildHistoryEvent, 0, len(in))
for _, ev := range in {
if ev == nil {
continue
}
events = append(events, ev)
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this out is to prevent holding a read lock on the build history during the iteration and function call? If that's the case, can you document why we read things into a temporary slice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean why not just update the same array in-place? I just thought it was cleaner this way and not a place where last bit of performance matters.

@tonistiigi tonistiigi merged commit 05a3313 into moby:master Mar 25, 2026
288 of 306 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants