-
Notifications
You must be signed in to change notification settings - Fork 27.8k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix: avoid race condition when download swc wasm #58536
Conversation
Allow CI Workflow Run
Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer |
1 similar comment
Allow CI Workflow Run
Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer |
Stats from current PRDefault BuildGeneral Overall increase
|
vercel/next.js canary | d3lm/next.js delm/fix/downloading-of-wasm | Change | |
---|---|---|---|
buildDuration | 10.7s | 10.5s | N/A |
buildDurationCached | 6s | 6.7s | |
nodeModulesSize | 199 MB | 199 MB | |
nextStartRea..uration (ms) | 429ms | 430ms | N/A |
Client Bundles (main, webpack)
vercel/next.js canary | d3lm/next.js delm/fix/downloading-of-wasm | Change | |
---|---|---|---|
199-HASH.js gzip | 28.7 kB | 28.7 kB | N/A |
3f784ff6-HASH.js gzip | 53.3 kB | 53.3 kB | ✓ |
494.HASH.js gzip | 180 B | 181 B | N/A |
framework-HASH.js gzip | 45.2 kB | 45.2 kB | ✓ |
main-app-HASH.js gzip | 242 B | 239 B | N/A |
main-HASH.js gzip | 31.7 kB | 31.7 kB | N/A |
webpack-HASH.js gzip | 1.7 kB | 1.7 kB | ✓ |
Overall change | 100 kB | 100 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | d3lm/next.js delm/fix/downloading-of-wasm | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | d3lm/next.js delm/fix/downloading-of-wasm | Change | |
---|---|---|---|
_app-HASH.js gzip | 194 B | 195 B | N/A |
_error-HASH.js gzip | 182 B | 181 B | N/A |
amp-HASH.js gzip | 501 B | 503 B | N/A |
css-HASH.js gzip | 322 B | 323 B | N/A |
dynamic-HASH.js gzip | 2.5 kB | 2.5 kB | ✓ |
edge-ssr-HASH.js gzip | 253 B | 255 B | N/A |
head-HASH.js gzip | 348 B | 347 B | N/A |
hooks-HASH.js gzip | 369 B | 368 B | N/A |
image-HASH.js gzip | 4.28 kB | 4.27 kB | N/A |
index-HASH.js gzip | 256 B | 256 B | ✓ |
link-HASH.js gzip | 2.61 kB | 2.6 kB | N/A |
routerDirect..HASH.js gzip | 311 B | 311 B | ✓ |
script-HASH.js gzip | 384 B | 383 B | N/A |
withRouter-HASH.js gzip | 307 B | 308 B | N/A |
1afbb74e6ecf..834.css gzip | 106 B | 106 B | ✓ |
Overall change | 3.17 kB | 3.17 kB | ✓ |
Client Build Manifests
vercel/next.js canary | d3lm/next.js delm/fix/downloading-of-wasm | Change | |
---|---|---|---|
_buildManifest.js gzip | 484 B | 483 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | d3lm/next.js delm/fix/downloading-of-wasm | Change | |
---|---|---|---|
index.html gzip | 529 B | 526 B | N/A |
link.html gzip | 540 B | 541 B | N/A |
withRouter.html gzip | 524 B | 522 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Edge SSR bundle Size
vercel/next.js canary | d3lm/next.js delm/fix/downloading-of-wasm | Change | |
---|---|---|---|
edge-ssr.js gzip | 92.4 kB | 92.4 kB | N/A |
page.js gzip | 145 kB | 145 kB | N/A |
Overall change | 0 B | 0 B | ✓ |
Middleware size
vercel/next.js canary | d3lm/next.js delm/fix/downloading-of-wasm | Change | |
---|---|---|---|
middleware-b..fest.js gzip | 626 B | 624 B | N/A |
middleware-r..fest.js gzip | 150 B | 151 B | N/A |
middleware.js gzip | 24.8 kB | 24.8 kB | N/A |
edge-runtime..pack.js gzip | 1.92 kB | 1.92 kB | ✓ |
Overall change | 1.92 kB | 1.92 kB | ✓ |
Next Runtimes
vercel/next.js canary | d3lm/next.js delm/fix/downloading-of-wasm | Change | |
---|---|---|---|
app-page-exp...dev.js gzip | 167 kB | 167 kB | ✓ |
app-page-exp..prod.js gzip | 93.4 kB | 93.4 kB | ✓ |
app-page-tur..prod.js gzip | 94.1 kB | 94.1 kB | ✓ |
app-page-tur..prod.js gzip | 88.7 kB | 88.7 kB | ✓ |
app-page.run...dev.js gzip | 137 kB | 137 kB | ✓ |
app-page.run..prod.js gzip | 88 kB | 88 kB | ✓ |
app-route-ex...dev.js gzip | 23.8 kB | 23.8 kB | ✓ |
app-route-ex..prod.js gzip | 16.4 kB | 16.4 kB | ✓ |
app-route-tu..prod.js gzip | 16.5 kB | 16.5 kB | ✓ |
app-route-tu..prod.js gzip | 16 kB | 16 kB | ✓ |
app-route.ru...dev.js gzip | 23.2 kB | 23.2 kB | ✓ |
app-route.ru..prod.js gzip | 16 kB | 16 kB | ✓ |
pages-api-tu..prod.js gzip | 9.37 kB | 9.37 kB | ✓ |
pages-api.ru...dev.js gzip | 9.64 kB | 9.64 kB | ✓ |
pages-api.ru..prod.js gzip | 9.37 kB | 9.37 kB | ✓ |
pages-turbo...prod.js gzip | 21.8 kB | 21.8 kB | ✓ |
pages.runtim...dev.js gzip | 22.5 kB | 22.5 kB | ✓ |
pages.runtim..prod.js gzip | 21.8 kB | 21.8 kB | ✓ |
server.runti..prod.js gzip | 49.1 kB | 49.1 kB | ✓ |
Overall change | 924 kB | 924 kB | ✓ |
Tests Passed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
probably a better way to convert the streams with Writable.toWeb
or Readable.fromWeb
, but this should work and I don't think it matters
What?
I noticed that code that was responsible for download the SWC Wasm fallback wasn't bullet proof and there was a chance for a race condition. The reason was that both
write
andclose
from a write stream are async operations and it's best to wait for them to complete, otherwise the promise returned frombody.pipeTo
could resolve before all data has been written and the stream was closed. Right after that it callsrename
and it could happen that, at that point,tempFile
doesn't contain all the data yet which means an empty file may be renamed into another file ending up withpath.join(cacheDirectory, tarFileName)
being empty as well.The fix is to wait for both
write
andclose
to be completed which eliminates the potential race condition between the fetch and the rename.