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

Wrong locking during more complex prunes #2337

Closed
alexlarsson opened this issue Apr 12, 2021 · 2 comments
Closed

Wrong locking during more complex prunes #2337

alexlarsson opened this issue Apr 12, 2021 · 2 comments
Labels
difficulty/medium medium complexity/difficutly issue

Comments

@alexlarsson
Copy link
Member

While doing some research into pruning I noticed that in ot-builtin-prune.c, if we take the more complex path that uses ostree_repo_prune_from_reachable() the exclusive repo lock is only held inside ostree_repo_prune_from_reachable(), so we compute the reachable set outside the lock.

If this where to race with a commit operation that created a new commit which we didn't traverse then it would be pruned away.

@cgwalters
Copy link
Member

Yeah, I think you're right. We clearly need a test that tries to reproduce this race. Also, fixing this would require fixing #2286

@cgwalters cgwalters added the difficulty/medium medium complexity/difficutly issue label Apr 15, 2021
cgwalters added a commit to cgwalters/ostree that referenced this issue Apr 15, 2021
Doing anything even somewhat sophisticated requires this;
turns out our own `ostree prune` CLI wants this, e.g.
ostreedev#2337

Closes: ostreedev#2286
cgwalters added a commit to cgwalters/ostree that referenced this issue Apr 15, 2021
Doing anything even somewhat sophisticated requires this;
turns out our own `ostree prune` CLI wants this, e.g.
ostreedev#2337

Closes: ostreedev#2286
dbnicholson pushed a commit to dbnicholson/ostree that referenced this issue Apr 29, 2021
Doing anything even somewhat sophisticated requires this;
turns out our own `ostree prune` CLI wants this, e.g.
ostreedev#2337

Closes: ostreedev#2286
dbnicholson pushed a commit to dbnicholson/ostree that referenced this issue Jun 5, 2021
Doing anything even somewhat sophisticated requires this;
turns out our own `ostree prune` CLI wants this, e.g.
ostreedev#2337

Closes: ostreedev#2286
dbnicholson pushed a commit to dbnicholson/ostree that referenced this issue Jun 5, 2021
Doing anything even somewhat sophisticated requires this;
turns out our own `ostree prune` CLI wants this, e.g.
ostreedev#2337

Closes: ostreedev#2286
dbnicholson pushed a commit to endlessm/ostree that referenced this issue Jun 7, 2021
Doing anything even somewhat sophisticated requires this;
turns out our own `ostree prune` CLI wants this, e.g.
ostreedev/ostree#2337

Closes: ostreedev/ostree#2286

(cherry picked from commit 0f36d8c)

The new symbols have been added to the released symbols file rather than
the development symbols file as in the upstream PR since our package
does a released build. Similarly, the upstream `LIBOSTREE_2021.3` symbol
version has been used so that packages don't need to be rebuilt when we
get to that version and drop our backported commits.

https://phabricator.endlessm.com/T31868
@jlebon
Copy link
Member

jlebon commented Feb 1, 2023

Oh hey, this is fixed by #2808!

@jlebon jlebon closed this as completed Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty/medium medium complexity/difficutly issue
Projects
None yet
Development

No branches or pull requests

3 participants