Skip to content

Conversation

tonistiigi
Copy link
Member

fixes #4419

When one flightcontrol callback gets canceled
ctx.Value() stops working for aquiring leases for
remaining callbacks. While this behavior should be also looked at more carefully, returning a lease for the first callback or for the remaining callback would not be correct as some objects can be tracked by first lease and that lease could be already deleted by the first callpath.

This fixes it so that any object tracked by flightcontrol callback will be copied to the lease of every codepath after the callback has returned.

@tonistiigi tonistiigi force-pushed the lease-flightcontrol-fix branch from 05abc3f to 55d2110 Compare January 8, 2024 06:56
@tonistiigi
Copy link
Member Author

@ktock Can you take a quick look at these "missing lease requirement" errors for stargz. I can easily remove this validation in Adopt() if leases are not needed for these functions but I don't want to hide any other possible bug if these code paths should actually have leases set in context. Atm. it seems to depend on the code path if a lease is set or not.

@ktock
Copy link
Collaborator

ktock commented Jan 9, 2024

Thanks for pinging me. According to the stack trace, the error comes from github.com/moby/buildkit/cache.(*immutableRef).prepareRemoteSnapshotsStargzMode and this function only creates snapshots that are traced by the ref's lease so I think it's OK the input ctx doesn't have lease.

When one flightcontrol callback gets canceled
ctx.Value() stops working for aquiring leases for
remaining callbacks. While this behavior should be
also looked at more carefully, returning a lease for
the first callback or for remaining callback would not
be correct as some objects can be tracked by first lease
and that lease could be already deleted by the first
callpath.

This fixes it so that any object tracked by flightcontrol
callback will be copied to the lease of every codepath
after the callback has returned.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi force-pushed the lease-flightcontrol-fix branch from 55d2110 to 4339ee5 Compare January 9, 2024 03:02
@AkihiroSuda AkihiroSuda requested a review from ktock January 23, 2024 08:50
@tonistiigi tonistiigi merged commit 2d608c3 into moby:master Jan 29, 2024
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.

unexpected error: missing lease requirement for setBlob
2 participants