From e0bab94c8ca679c616701c6ffb6fa19d4eaa414c Mon Sep 17 00:00:00 2001 From: piotr Date: Tue, 22 Sep 2026 18:27:04 +0000 Subject: [PATCH] history: remove ref count entry when last listener exits Listen with a Ref filter increments h.refs[ref] and decrements it on exit, but only removed the map entry when the ref had already been marked deleted. A listener that exits normally therefore left a zero-valued entry behind for the lifetime of the process. delete() treats the presence of the key as "a listener is still streaming this record" and only marks the ref in h.deleted, deferring the physical removal to the last listener's exit. With no listener left, that never happens: the record stays in the history database and its lease is kept, UpdateBuildHistory{Delete} reports success but is a no-op, a later Listen on the ref fails with "ref is deleted", and the history GC (MaxAge/MaxEntries and orphan cleanup) cannot remove the record until buildkitd restarts. buildx calls ListenBuildHistory with the build's ref after every build that writes a metadata file (provenance resolution), so any such record becomes undeletable. On per-job sticky disks the record survives in history.db across the commit and is re-exported by the next job. Remove the map entry whenever the count reaches zero, and run the pending delete if the ref was marked deleted in the meantime, so that the invariant "key present in refs <=> active listener" holds. Add an integration test that listens on a ref to completion, deletes the ref and verifies the record is gone and the DELETED event is emitted. Same change as the upstream branch history-release-listener-refs. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- client/client_test.go | 75 ++++++++++++++++++++++++ solver/llbsolver/history/buildhistory.go | 6 +- 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 6700a0c58611..b16fd9aa92b7 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -264,6 +264,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){ testSourcePolicySignedCommit, testSourcePolicySessionHTTPChecksumAssist, testSourcePolicySessionConvert, + testDeleteBuildHistoryAfterListenerExit, testListenBuildHistoryExcludesSoftDeletedRecords, } @@ -13605,6 +13606,80 @@ func testListenBuildHistoryExcludesSoftDeletedRecords(t *testing.T, sb integrati } } +func testDeleteBuildHistoryAfterListenerExit(t *testing.T, sb integration.Sandbox) { + c, err := New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + def, err := llb.Scratch().File(llb.Mkfile("file", 0o644, nil)).Marshal(sb.Context()) + require.NoError(t, err) + + ref := identity.NewID() + _, err = c.Solve(sb.Context(), def, SolveOpt{Ref: ref}, nil) + require.NoError(t, err) + + // Listen on the ref and let the stream finish, as buildx does when + // resolving provenance after a build. Draining to EOF guarantees the + // server-side listener has exited before the delete below. + cl, err := c.ControlClient().ListenBuildHistory(sb.Context(), &controlapi.BuildHistoryRequest{ + Ref: ref, + EarlyExit: true, + }) + require.NoError(t, err) + _, err = cl.Recv() + require.NoError(t, err) + _, err = cl.Recv() + require.ErrorIs(t, err, io.EOF) + + // A streaming listener with no ref filter observes the DELETED event + // only when the record is actually removed. Reading the initial record + // ensures the listener is subscribed before the delete is issued. + eventsCtx, eventsCancel := context.WithCancelCause(sb.Context()) + defer eventsCancel(nil) + events, err := c.ControlClient().ListenBuildHistory(eventsCtx, &controlapi.BuildHistoryRequest{}) + require.NoError(t, err) + ev, err := events.Recv() + require.NoError(t, err) + require.Equal(t, ref, ev.Record.Ref) + + _, err = c.ControlClient().UpdateBuildHistory(sb.Context(), &controlapi.UpdateBuildHistoryRequest{ + Ref: ref, + Delete: true, + }) + require.NoError(t, err) + + // With no listener left the delete must be immediate, not deferred: the + // ref is neither reported as pending deletion nor present in the store. + history, err := c.ControlClient().ListenBuildHistory(sb.Context(), &controlapi.BuildHistoryRequest{ + Ref: ref, + EarlyExit: true, + }) + require.NoError(t, err) + _, err = history.Recv() + require.ErrorIs(t, err, io.EOF) + + deleted := make(chan error, 1) + go func() { + for { + ev, err := events.Recv() + if err != nil { + deleted <- err + return + } + if ev.Type == controlapi.BuildHistoryEventType_DELETED && ev.Record.Ref == ref { + deleted <- nil + return + } + } + }() + select { + case err := <-deleted: + require.NoError(t, err) + case <-time.After(10 * time.Second): + t.Fatalf("no DELETED event received for ref %s", ref) + } +} + func parseFSMetadata(t *testing.T, dt []byte) []fsutiltypes.Stat { var m []fsutiltypes.Stat for len(dt) > 0 { diff --git a/solver/llbsolver/history/buildhistory.go b/solver/llbsolver/history/buildhistory.go index 2e43bae660d2..19916152f165 100644 --- a/solver/llbsolver/history/buildhistory.go +++ b/solver/llbsolver/history/buildhistory.go @@ -784,9 +784,9 @@ func (h *Queue) Listen(ctx context.Context, req *controlapi.BuildHistoryRequest, defer func() { h.mu.Lock() h.refs[req.Ref]-- - if _, ok := h.deleted[req.Ref]; ok { - if h.refs[req.Ref] == 0 { - delete(h.refs, req.Ref) + if h.refs[req.Ref] == 0 { + delete(h.refs, req.Ref) + if _, ok := h.deleted[req.Ref]; ok { h.delete(req.Ref) } }