fs.read race condition when using Promise.all

[oleiade]

Description

The current implementation of the experimental/fs module read (and potentially seek) method seems vulnerable to race conditions under specific circumstances. Below are the issues identified:

1. Concurrent reads on the same buffer can lead to unpredictable behavior

let file = fs.open(name);
let buffer = new Uint8Array(5);
let p1 = file.read(buffer);
let p2 = file.read(buffer);
await Promise.all([p1, p2]);

2. A buffer modification prior to the resolution of a promise can lead to uncertainty about the final state of the buffer:

let file = fs.open(name);
let buffer = new Uint8Array(5);
let p1 = file.read(buffer);
buffer[2] = 3;
console.log(buffer); 
await p1;

In this case, it's unclear if the buffer will hold the data read from the file, just the third value 3, or a combination of both.

This behavior seems inconsistent with the promise paradigm, where state changes should only be observable once the promise resolves.

Additional Observations

Preliminary tests with Deno suggest that:

• Sequential read calls populate data once one of the promises is awaited.
• Awaiting a promise from an already completed read doesn't affect the buffer.
• The buffer remains unchanged if a read promise isn't awaited.

Reviewing how Deno and Node handle such situations might be beneficial for insight.

Concerns

While the current behavior might be acceptable for an experimental module, allowing race conditions in stable code could lead to unpredictable behavior and potential bugs. We expect this issue to be addressed as a prerequisite for the experimental/fs module to become stable.

References

#2977
#3141
#3309
#3145
#3148

[oleiade]

This was addressed by #3309

Here's a related follow-up issue to keep tracking: #3433