Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: Don't close base images prematurely #455

Merged
merged 1 commit into from
May 21, 2024

Conversation

lmbarros
Copy link
Collaborator

@lmbarros lmbarros commented Mar 29, 2024

This fixes a bug introduced in the refactoring made during the work on Delta on load. The previous implementation was closing the base image stream before it was even used.

This effectively marked layers as no longer locked, which means other concurrent tasks would be able to remove them while they were still needed. So, this PR is very likely to help with #444 -- possibly solving it entirely (though it's hard to test or otherwise confirm this with 100% certainty).

The solution implemented here is simple enough: we make it clear who is responsible for closing this stream, and ensure it is closed only after it's used.

Our previous implementation was closing the base image stream before it
was even used. With this commit we make it clear who is responsible for
closing this stream, and ensure it is closed only after it's used.

Signed-off-by: Leandro Motta Barros <leandro@balena.io>
Change-type: patch
@lmbarros lmbarros self-assigned this Mar 29, 2024
@lmbarros lmbarros marked this pull request as ready for review May 20, 2024 22:11
@alexgg
Copy link
Contributor

alexgg commented May 21, 2024

lgtm

@alexgg alexgg merged commit 56a9b7f into master May 21, 2024
49 checks passed
@alexgg alexgg deleted the lmb/bugfix-delta-stream-closing branch May 21, 2024 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants