Close config files right after read - #1317
Draft
davidanthoff wants to merge 9 commits into
Draft
Conversation
…g/juliaup into close-config-files-more
IanButterworth
added a commit
that referenced
this pull request
Jun 6, 2026
Shrink the region covered by the global juliaup configuration lock so that
the network download/extract of a new version no longer happens while the
exclusive lock is held. `juliaup add` now:
1. takes the shared lock only briefly to check whether the channel is
already installed, then releases it;
2. downloads and extracts the version into a temporary directory inside
juliauphome with NO lock held;
3. re-acquires the exclusive lock only to atomically rename the download
into place and update the config.
This splits the old `install_version` into `download_version_to_temp`
(lock-free, network-bound) and `commit_version_install` (lock-held, fast
rename + config write). `install_version` is retained as a thin wrapper for
existing callers.
How this honors the discussion (#435, #561):
- Implements davidanthoff's proposed design from #435: read config + release
lock, download, then re-acquire and commit, instead of holding the write
lock for the whole download.
- Honors LilithHafner's refinement: the commit phase re-checks whether the
install is still needed, so two processes installing the same version in
parallel resolve cleanly (the loser discards its temp dir) rather than
erroring.
- Keeps StefanKarpinski's shared/exclusive model: many readers (e.g. the
launcher checking for self-update / auto-install) coexist and only block
for the millisecond-scale commit, not for a multi-minute download.
Benefit over the other open PRs:
- vs #1238 (remove file locking entirely, atomic-rename only): juliaup
performs multi-file operations (install dir + config + symlinks) that a
single-file atomic rename cannot keep mutually consistent. This change
retains the lock for cross-file consistency while still eliminating the
long-lock pathology that #1238 targets, addressing davidanthoff's stated
objection to dropping the lock outright.
- vs #1317 (close config files right after read): complementary rather than
competing — that PR shortens how long handles stay open; this PR shortens
how long the exclusive lock is held during the expensive download. They
compose cleanly.
IanButterworth
added a commit
that referenced
this pull request
Jun 10, 2026
Shrink the region covered by the global juliaup configuration lock so that
the network download/extract of a new version no longer happens while the
exclusive lock is held. `juliaup add` now:
1. takes the shared lock only briefly to check whether the channel is
already installed, then releases it;
2. downloads and extracts the version into a temporary directory inside
juliauphome with NO lock held;
3. re-acquires the exclusive lock only to atomically rename the download
into place and update the config.
This splits the old `install_version` into `download_version_to_temp`
(lock-free, network-bound) and `commit_version_install` (lock-held, fast
rename + config write). `install_version` is retained as a thin wrapper for
existing callers.
How this honors the discussion (#435, #561):
- Implements davidanthoff's proposed design from #435: read config + release
lock, download, then re-acquire and commit, instead of holding the write
lock for the whole download.
- Honors LilithHafner's refinement: the commit phase re-checks whether the
install is still needed, so two processes installing the same version in
parallel resolve cleanly (the loser discards its temp dir) rather than
erroring.
- Keeps StefanKarpinski's shared/exclusive model: many readers (e.g. the
launcher checking for self-update / auto-install) coexist and only block
for the millisecond-scale commit, not for a multi-minute download.
Benefit over the other open PRs:
- vs #1238 (remove file locking entirely, atomic-rename only): juliaup
performs multi-file operations (install dir + config + symlinks) that a
single-file atomic rename cannot keep mutually consistent. This change
retains the lock for cross-file consistency while still eliminating the
long-lock pathology that #1238 targets, addressing davidanthoff's stated
objection to dropping the lock outright.
- vs #1317 (close config files right after read): complementary rather than
competing — that PR shortens how long handles stay open; this PR shortens
how long the exclusive lock is held during the expensive download. They
compose cleanly.
Member
|
I think I closed this accidentally. Thought it was #746 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I think this probably should work because we now have a separate lock file that handles all concurrency.
WIP at the moment.