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

fix: avoid chmod on read stream close #2096

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Oct 2, 2023

Summary:

Recently the Babel react-native e2e test is breaking:

https://github.com/babel/babel/actions/runs/6345671251/job/17316512072#step:10:82

Error: ENOENT: no such file or directory, chmod '/tmp/rnbabel/ios/HelloWorld/Images.xcassets/Contents.json'
    at Object.chmodSync (node:fs:1991:3)
    at Object.chmodSync (/home/runner/.npm/_npx/[79](https://github.com/babel/babel/actions/runs/6345671251/job/17316512072#step:10:80)30a8670f922cdb/node_modules/graceful-fs/polyfills.js:263:21)
    at ReadStream.<anonymous> (/home/runner/.npm/_npx/7930a8670f922cdb/node_modules/@react-native-community/cli/build/tools/copyFiles.js:83:19)
    at ReadStream.emit (node:events:517:28)
    at emitCloseNT (node:internal/streams/destroy:132:10)
    at process.processTicksAndRejections (node:internal/process/task_queues:81:21)

The exact ENOENT file seems random, e.g. it could also be /tmp/rnbabel/App.tsx in another CI job result. It occurs in other projects unrelated to Babel as well: akveo/react-native-ui-kitten#1630

The issue is likely due to a race condition in

readStream.on('close', () => {
done();
fs.chmodSync(destPath, mode);
});

When the read stream is closed, the write stream might not have been closed yet so chmod is racing with the write stream.

Here we avoid the chmodSync call and instead provide the desired mode upfront to the write stream so that the copied file will inherit the source file's mode.

Test Plan:

Manual test.

Create a file named ./test.js

'use strict';
const fs = require('node:fs');
function copyBinaryFile(srcPath, destPath, cb) {
  let cbCalled = false;
  const {mode} = fs.statSync(srcPath);
  const readStream = fs.createReadStream(srcPath);
  const writeStream = fs.createWriteStream(destPath, {mode});
  readStream.on('error', (err) => {
    done(err);
  });
  writeStream.on('error', (err) => {
    done(err);
  });
  readStream.on('close', () => {
    done();
  });
  readStream.pipe(writeStream);
  function done(err) {
    if (!cbCalled) {
      cb(err);
      cbCalled = true;
    }
  }
}

copyBinaryFile('./foo', './bar', () => {});

Run the following commands in shell

touch ./foo
echo "foo" > ./foo
chmod 0400 ./foo
node ./test.js
stat ./bar

Expect: The ./bar should be created with 0400 mode with the content "foo"

Checklist

  • Documentation is up to date to reflect these changes.
  • Follows commit message convention described in CONTRIBUTING.md

Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Thank you! We also hit this flakiness within our own fs-based tests

@thymikee thymikee merged commit 0af4af6 into react-native-community:main Oct 3, 2023
6 checks passed
@JLHwung JLHwung deleted the avoid-chmod-on-read-stream-close branch October 3, 2023 16:05
@atlj
Copy link
Contributor

atlj commented Oct 5, 2023

Thank you, we also hit this issue a lot on react-native-builder-bob's CI

@Productivix
Copy link

Productivix commented Oct 12, 2023

Hi,
I create 2 or 3 RTN packages /week on Linux machines : this bug is a very blocking one : do you have an immediate manual solution please ?

@atlj
Copy link
Contributor

atlj commented Oct 12, 2023

@Productivix if you won't mind using a release candidate version, you could try:

npx create-react-native-library@latest myLib --react-native-version 0.73.0-rc.2

@thymikee
Copy link
Member

thymikee commented Oct 13, 2023

Ported this to 11.3.9, which the core team can pick for 0.72 point release (as requested in babel/babel#16024 (comment))

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.

None yet

4 participants