Skip to content

fix(download): make concurrent first-run downloads safe - #555

Merged
Cloak-HQ merged 7 commits into
CloakHQ:mainfrom
Shub3am:fix/download-concurrent-extract
Oct 7, 2026
Merged

Cloak-HQ merged 7 commits into
CloakHQ:mainfrom
Shub3am:fix/download-concurrent-extract

Conversation

@Shub3am

@Shub3am Shub3am commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Problem

When several processes or threads start on an empty cache, they all download the binary and extract it. _extract_archive (and its JS and .NET ports) deletes the install dir and then extracts into it in place. That leaves two failure windows:

  • A second caller deletes the first caller's install while it is still being extracted.
  • A caller passes the "binary exists and is executable" check on a binary that is still being written, and returns it.

This is common in CI and in worker pools that launch N browsers at once on a fresh machine.

Reproduced against a local mirror of the release archive, with 4 processes started together on an empty cache, before this change:

Client / OS Result
JS, macOS 0/4 launched. ENOTEMPTY, Directory not empty, dlopen ... Chromium Framework (no such file). The cache was left with 223 of 917 entries, and it stays broken for later runs because the binary itself exists.
Python, macOS Failures in every staggered run: OSError: [Errno 66] Directory not empty, dlopen failures. The install was left at 601 or 909 of 917 entries.
JS, Linux (Docker) spawnSync .../chrome ETXTBSY: the binary was executed while another caller was still writing it.

Fix

Same change in Python, JS and .NET:

  • Extract into a uniquely named sibling staging dir (<install>.partial-<random hex>) in the same cache dir, then rename it into place. The rename is atomic on the same filesystem.
  • If the destination already holds a complete install when the rename happens, another caller won. The staged copy is discarded and that install is used.
  • An incomplete leftover from an interrupted run is still replaced. I rename it aside to <install>.stale-<random> and delete it from there, then retry the rename into place (up to 5 attempts, re-checking for a complete install each time). Deleting it in place raced other callers renaming their extraction into the same path.
  • Windows only: a directory rename that fails with access denied (antivirus or the indexer briefly holding freshly written files open) is retried for about 2.5 seconds while the destination does not exist. Python catches PermissionError, JS EPERM/EACCES/EBUSY, .NET UnauthorizedAccessException or an IOException with the access denied, sharing violation or lock violation HRESULT.
  • Cleanup of the staging and stale dirs is best effort, so a failed delete never masks the error being raised.
  • JS only: the temporary archive name now uses randomUUID() instead of Date.now(), which collided for calls made in the same millisecond.

No new config, and no behaviour change on the single-caller path.

Tests

New regression tests in each client start several concurrent first-run downloads against a local server and assert that:

  • every caller gets a binary path,
  • the install is complete every time a caller observes it,
  • no staging dirs are left behind,
  • a partial install from an interrupted extraction is replaced, including when several callers find it at once (JS runs these callers as separate processes, since in-process callers cannot interleave inside the synchronous rename).

Before the fix each new test fails:

Python: 1 failed  ("raised", "FileNotFoundError(2, 'No such file or directory')")
JS:     1 failed  (expected observedStates to equal ["complete"])
.NET:   1 failed  (System.IO.IOException: The file '.../lib/part-0.bin' already exists, from Download.ExtractTar)

Before the leftover-replacement fix, the concurrent partial install tests fail every run (5/5 Python, 8/8 JS, 5/5 .NET):

Python: ("raised", "FileNotFoundError(2, 'No such file or directory')")
JS:     ENOTEMPTY: directory not empty, rename '...chromium-145.0.7632.109.2.partial-<uuid>' -> '...chromium-145.0.7632.109.2'
.NET:   IOException: Directory not empty

After it, they passed 10/10 runs in each client.

After the fix, full suites:

Suite Result
pytest -m "not slow" 761 passed, 1 skipped
vitest run 606 passed, 19 skipped
dotnet build + dotnet test (sdk 8.0, same commands as CI) 487 passed, 1 skipped, 0 failed

With the fix, the cross-process repro launched 4/4 and 6/6 processes on an empty cache, each leaving a complete 917-entry install.

Notes for review

  • The staging dir sits next to the install dir, not in the system temp dir, so the rename never crosses filesystems.
  • The Windows retry is not exercised in CI, which runs on Ubuntu. The new concurrent tests skip on Windows.
  • Concurrent callers still each download the archive. I kept this change to making the result correct. A download lock would be a separate change if you want it.

Shub3am and others added 7 commits September 23, 2026 15:40
_extract_archive deleted the install dir and then extracted into it in
place. When several processes or threads start on an empty cache, a
second caller can delete the first caller's install mid-extraction, or
pass the "binary exists and is executable" check on a binary that is
still being written. On Linux that surfaces as ETXTBSY on spawn; on
macOS as dlopen failures and "Directory not empty", and the cache can
be left permanently incomplete.

Extract into a uniquely named sibling staging dir and rename it into
place. If the destination already holds a complete install, another
caller won and the staged copy is discarded; an incomplete leftover is
still replaced as before.
Same race as the Python wrapper: extractArchive deleted the install dir
and extracted into it in place, so concurrent ensureBinary() calls on an
empty cache could delete each other's install or return a binary that
was still being written (spawn ETXTBSY on Linux).

Extract into a uniquely named sibling staging dir and rename it into
place, keeping an existing complete install if another caller won.
Also name the temporary archive with randomUUID() instead of
Date.now(), which collided for calls made in the same millisecond.
Same race as the Python and JS wrappers: ExtractArchive deleted the
install dir and extracted into it in place, so concurrent
EnsureBinaryAsync() calls on an empty cache failed with "already
exists" / DirectoryNotFoundException or saw a half-written install.

Extract into a uniquely named sibling staging dir and move it into
place, keeping an existing complete install if another caller won.
When an incomplete install dir was left over from an interrupted run,
every concurrent caller saw it as incomplete and deleted it in place.
The first to finish renamed its staging dir into place; the others'
unguarded second rename then failed (ENOTEMPTY, FileNotFoundError,
DirectoryNotFoundException), and a late delete could remove files from
an install another caller had just moved in. With 8 concurrent callers,
7 raised in each round.

Rename the leftover aside to a unique <install>.stale-<id> dir and
delete it from there, then retry the staging rename a bounded number
of times, keeping a complete install if another caller got there
first. Applies to the Python, JS and .NET clients, each with a
regression test that starts several concurrent callers on a partial
install. The JS test uses separate processes, since callers in one
process cannot interleave inside the synchronous rename block.
Antivirus and indexers on Windows briefly hold freshly extracted files
open, which fails the directory rename with access denied. Retry that
rename for about 2.5 seconds while the destination does not exist, in
all three clients. Cleanup of the staging and stale dirs is now best
effort so a failed delete never masks the error being raised.
…rent-extract

# Conflicts:
#	js/src/download.ts
@Cloak-HQ
Cloak-HQ merged commit 9fb05e0 into CloakHQ:main Oct 7, 2026
5 of 6 checks passed
@Cloak-HQ

Cloak-HQ commented Oct 7, 2026

Copy link
Copy Markdown
Member

Thanks for this, @Shub3am. Really solid work.

We resolved a small conflict with main (the PRO_MAJOR bump in download.ts) and merged it.
Before merging we also ran a concurrent first-run test on a real Windows machine with Defender on, and every caller got a complete install with no leftover staging dirs.

The fix will ship in the next wrapper release for Python, JS and .NET.

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.

2 participants