Skip to content

Conversation

@blampe
Copy link
Contributor

@blampe blampe commented Nov 14, 2025

I suspect (but can't confirm) the issue in #6527 is due to a race condition when writing the downloaded archive to disk.

If the write isn't fully flushed to disk, it's possible for readers to see an incomplete/invalid archive. As a precaution, we should explicitly flush before returning.

The issue is probably more likely on CI runners with slower file systems.

As for why we seem to only see this with yarn, I'm guessing its smaller size (1.2M) makes it more likely to sit in OS buffers longer.

This is all just a theory! Again I haven't been able to reliably reproduce the issue. Adding a flush seems like it can't hurt, though.


Note

Flushes the downloaded file to disk with file.sync_all() after write to prevent incomplete reads.

Written by Cursor Bugbot for commit ec82db4. This will update automatically on new commits. Configure here.

I suspect (but can't confirm) the issue in
jdx#6527 is due to a race condition
when writing the downloaded archive to disk.

If the write isn't fully flushed to disk, it's possible for readers to
see an incomplete/invalid archive. As a precaution, we should explicitly
flush before returning.

The issue is probably more likely on CI runners with slower file
systems.

As for why we seem to only see this with `yarn`, I'm guessing its
smaller size (1.2M) makes it more likely to sit in OS buffers longer.

This is all just a theory! Again I haven't been able to reliably
reproduce the issue. Adding a flush seems like it can't hurt, though.
Copilot AI review requested due to automatic review settings November 14, 2025 04:49
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds an explicit flush operation to the vfox download function to prevent potential race conditions when writing downloaded archives to disk. The change addresses issue #6527 where incomplete file writes might cause archive corruption, particularly on CI systems with slower file systems or for smaller files like yarn (1.2MB) that may remain in OS buffers longer.

  • Adds sync_all() call after writing downloaded bytes to disk to ensure complete flush before returning

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@blampe blampe changed the title Flush vfox download buffer fix: Flush vfox download buffer Nov 14, 2025
@jdx
Copy link
Owner

jdx commented Nov 14, 2025

bugbot run

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no bugs!


@jdx jdx merged commit 0147a76 into jdx:main Nov 14, 2025
37 of 38 checks passed
jdx pushed a commit that referenced this pull request Nov 15, 2025
### 🚀 Features

- **(http)** Add 'format' to http backend by @thejcannon in
[#6957](#6957)

### 🐛 Bug Fixes

- **(bootstrap)** wrong directory on first run by @vmeurisse in
[#6971](#6971)
- **(tasks)** fix nested colons with `mise task edit` by @jdx in
[#6978](#6978)
- Use compatible env flags by @thejcannon in
[#6964](#6964)
- Flush vfox download buffer by @blampe in
[#6969](#6969)

### 📚 Documentation

- `arch()` template is `x64` by @thejcannon in
[#6967](#6967)
- update section headers in getting-started.md by @JunichiroKohari in
[#6980](#6980)

### New Contributors

- @JunichiroKohari made their first contribution in
[#6980](#6980)
- @blampe made their first contribution in
[#6969](#6969)
- @thejcannon made their first contribution in
[#6964](#6964)

## 📦 Aqua Registry Updates

#### New Packages (1)

- [`cirruslabs/cirrus-cli`](https://github.com/cirruslabs/cirrus-cli)

#### Updated Packages (1)

- [`axodotdev/cargo-dist`](https://github.com/axodotdev/cargo-dist)
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