Skip to content

libstore: only acquire big write lock when needed, hold read lock#15967

Open
lisanna-dettwyler wants to merge 1 commit into
NixOS:masterfrom
lisanna-dettwyler:local-store-fix-locking
Open

libstore: only acquire big write lock when needed, hold read lock#15967
lisanna-dettwyler wants to merge 1 commit into
NixOS:masterfrom
lisanna-dettwyler:local-store-fix-locking

Conversation

@lisanna-dettwyler
Copy link
Copy Markdown
Contributor

@lisanna-dettwyler lisanna-dettwyler commented Jun 3, 2026

Check if a schema migration is actually needed before acquiring exclusive on the big nix store lock, and always hold a shared lock throughout the duration of the LocalStore. Checking if it's needed prevents the build hook from trying to acquire exclusive while the parent is holding shared.

Motivation

We need to hold the shared lock throughout the lifetime of the LocalStore to prevent other schema upgrades from happening while we're using the store.

Context

Prior attempt to fix this logic: #15941

There are no schema migration tests that I know of, so this logic should be thoroughly examined before merging, although keeping in mind that the current state of affairs is already broken and can result in a hang.


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@github-actions github-actions Bot added the store Issues and pull requests concerning the Nix store label Jun 3, 2026
@lisanna-dettwyler
Copy link
Copy Markdown
Contributor Author

I might also be able to remove the buggyNeedLocalStore from concurrent-builds.sh with this, testing now.

@github-actions github-actions Bot added the with-tests Issues related to testing. PRs with tests have some priority label Jun 3, 2026
Comment thread src/libstore/local-store.cc Outdated
@lisanna-dettwyler lisanna-dettwyler force-pushed the local-store-fix-locking branch 2 times, most recently from 19c62b4 to b260647 Compare June 5, 2026 21:03
Check if a schema migration is actually needed before acquiring
exclusive on the big nix store lock, and always hold a shared lock
throughout the duration of the LocalStore. Checking if it's needed
prevents the build hook from trying to acquire exclusive while the
parent is holding shared.

Signed-off-by: Lisanna Dettwyler <lisanna.dettwyler@gmail.com>
@lisanna-dettwyler lisanna-dettwyler force-pushed the local-store-fix-locking branch from b260647 to 6d8846a Compare June 5, 2026 21:06
Comment thread src/libstore/local-store.cc
@lisanna-dettwyler lisanna-dettwyler force-pushed the local-store-fix-locking branch 2 times, most recently from 22ed4a8 to 41e2fab Compare June 6, 2026 00:34
@lisanna-dettwyler
Copy link
Copy Markdown
Contributor Author

Dropping the buggyNeedLocalStore removal for now because I don't want it to hold this up. For some reason removing that passes in checks.x86_64-linux.nix-functional-tests but hangs in hydraJobs.installTests.x86_64-linux.againstSelf, but I don't know the difference between these two.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

store Issues and pull requests concerning the Nix store with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants