Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs.createWriteStream can cause out-of-order writes, in v18.16+ #51993

Closed
dy-dx opened this issue Mar 6, 2024 · 3 comments
Closed

fs.createWriteStream can cause out-of-order writes, in v18.16+ #51993

dy-dx opened this issue Mar 6, 2024 · 3 comments
Assignees
Labels
confirmed-bug Issues with confirmed bugs.

Comments

@dy-dx
Copy link

dy-dx commented Mar 6, 2024

Version

v18.16.0 and up, v19.8.0 and up, v20.x, v21.x

Platform

Darwin x86_64

Subsystem

fs, stream

What steps will reproduce the bug?

const fs = require('fs');

const w = fs.createWriteStream('file.txt');

w.on('open', () => {
  w.write('hello');

  process.nextTick(() => {
    w.write('world');
    w.end();
  });
});

w.on('close', () => {
  console.log(fs.readFileSync('file.txt', 'utf8'));
});

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior? Why is that the expected behavior?

helloworld

What do you see instead?

worldhello

Additional information

The issue was caused by this change: #46818, which landed in node v18.16.0 and v19.8.0.
(Thanks to @Fadorico for tracking down this change. I have also confirmed this by building node locally.)

This is causing the 'unzipper' module to create corrupted files, because its 'fstream' dependency uses fs.createWriteStream() like in the example above.

It looks like when the stream's "open" event fires, the stream does not yet have the "constructed" flag set, so that might be why things are getting weird.

As a workaround, if I wait an additional tick after the "open" event, there are no issues:

w.on('open', () => {
  process.nextTick(() => {
    w.write('hello');

    process.nextTick(() => {
      w.write('world');
      w.end();
    });
  });
});
@dy-dx dy-dx changed the title fs.createWriteStream can cause out-of-order writes, in v18.16+ and v19.8+ fs.createWriteStream can cause out-of-order writes, in v18.16+ Mar 6, 2024
@benjamingr
Copy link
Member

@mcollina @ronag

@mcollina mcollina added the confirmed-bug Issues with confirmed bugs. label Mar 7, 2024
@mcollina mcollina self-assigned this Mar 7, 2024
@mcollina
Copy link
Member

mcollina commented Mar 7, 2024

I'm sorry it caused a regression this far down. Will patch it. Hopefully we can backport the fix to v18.

mcollina added a commit to mcollina/node that referenced this issue Mar 7, 2024
@targos targos closed this as completed in cb85073 Mar 8, 2024
targos pushed a commit that referenced this issue Mar 8, 2024
Fixes: #51993
PR-URL: #52005
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
richardlau pushed a commit that referenced this issue Mar 23, 2024
Fixes: #51993
PR-URL: #52005
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
richardlau pushed a commit that referenced this issue Mar 25, 2024
Fixes: #51993
PR-URL: #52005
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
richardlau pushed a commit that referenced this issue Mar 25, 2024
Fixes: #51993
PR-URL: #52005
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
rdw-msft pushed a commit to rdw-msft/node that referenced this issue Mar 26, 2024
Fixes: nodejs#51993
PR-URL: nodejs#52005
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@dy-dx
Copy link
Author

dy-dx commented Mar 26, 2024

I can confirm the issue has been fixed in the following node.js versions:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants