Skip to content

fix(solver): check for context canceled in ReleaseUnreferenced - #7236

Open
jsternberg wants to merge 1 commit into
moby:masterfrom
jsternberg:prune-release-unreferenced-on-canceled-context
Open

jsternberg wants to merge 1 commit into
moby:masterfrom
jsternberg:prune-release-unreferenced-on-canceled-context

Conversation

@jsternberg

@jsternberg jsternberg commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

The existing implementations do not check for the context being canceled
when ReleaseUnreferenced is called which has caused a bug in how the
controller invokes ReleaseUnreferenced to be uncaught. We pass an
already canceled context to the method which causes any implementations
that do check the context to always fail.

@jsternberg

Copy link
Copy Markdown
Collaborator Author

Waiting to verify that the CI tests fail properly when the context is checked in the existing code and then I'll amend the commit with the fix.

Found this while working on the sqlite cache backend since the sqlite version of ReleaseUnreferenced honored the context cancellation and caused the error to trigger.

#7216

@jsternberg
jsternberg force-pushed the prune-release-unreferenced-on-canceled-context branch from d6d7917 to 9c05191 Compare September 30, 2026 18:56
@jsternberg

Copy link
Copy Markdown
Collaborator Author

Coming back to this and after talking briefly with @tonistiigi I'm just going to submit the fix here and not bother with tests. Testing this behavior adds far too much code and overhead than is gained from the tests themselves. Since the call to ReleaseUnreferenced here logs instead of failing and I don't want to change that, I'd have to add or change a test to detect when the log message was printed or I'd have to add some mock implementations. None of that is very helpful for such a simple change.

@jsternberg
jsternberg force-pushed the prune-release-unreferenced-on-canceled-context branch from 9c05191 to 30b196c Compare October 9, 2026 16:42
@jsternberg
jsternberg requested a review from tonistiigi October 9, 2026 16:42
@jsternberg
jsternberg marked this pull request as ready for review October 9, 2026 16:46
Comment thread control/control.go Outdated
The existing implementations do not check for the context being canceled
when `ReleaseUnreferenced` is called which has caused a bug in how the
controller invokes `ReleaseUnreferenced` to be uncaught. We pass an
already canceled context to the method which causes any implementations
that do check the context to always fail.

Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
@jsternberg
jsternberg force-pushed the prune-release-unreferenced-on-canceled-context branch from 30b196c to b853c78 Compare October 9, 2026 18:18
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.

2 participants