mirror of
https://github.com/nodejs/node.git
synced 2026-10-28 18:25:13 +08:00
vfs: keep open memory handles in sync with file
A MemoryFileHandle snapshotted the file's buffer at open and never checked the entry again. After the file changed through the provider or another handle, an open handle served stale reads and fstat sizes, and its writes went into the old copy, undoing other handles' changes. Record the entry content each handle last saw or wrote, and adopt the entry's content whenever another handle has replaced it, so an open descriptor is a live view of the file as on a real filesystem. Signed-off-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Assisted-by: claude:opus-5.5 PR-URL: https://github.com/nodejs/node/pull/66356 Fixes: https://github.com/nodejs/node/issues/66355 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit is contained in:
1 parent
a7e06d975d
commit
44d5d57ad2
2 files changed
+117
No files matched your search
@@ -431,6 +431,7 @@ class MemoryFileHandle extends VirtualFileHandle {
|
||||
#entry;
|
||||
#getStats;
|
||||
#isReadOnly;
|
||||
#published; // The entry content this handle last saw or wrote.
|
||||
|
||||
#checkClosed(syscall) {
|
||||
if (this.closed) {
|
||||
@@ -454,6 +455,7 @@ class MemoryFileHandle extends VirtualFileHandle {
|
||||
this.#entry = entry;
|
||||
this.#getStats = getStats;
|
||||
this.#isReadOnly = isReadOnly;
|
||||
this.#published = content;
|
||||
|
||||
// O_TRUNC empties the file at open time. O_APPEND does not move the
|
||||
// read offset: it only forces writes to the end, so the position stays
|
||||
@@ -463,10 +465,24 @@ class MemoryFileHandle extends VirtualFileHandle {
|
||||
this.#size = 0;
|
||||
if (entry) {
|
||||
entry.content = this.#content;
|
||||
this.#published = this.#content;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Adopts the entry's content if another handle replaced it, since an
|
||||
* open descriptor is a live view of the file, not a snapshot.
|
||||
*/
|
||||
#sync() {
|
||||
const entry = this.#entry;
|
||||
if (entry && entry.content !== this.#published) {
|
||||
this.#content = entry.content;
|
||||
this.#size = entry.content.length;
|
||||
this.#published = entry.content;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Throws EBADF if the handle was not opened for writing, or EROFS if the
|
||||
* provider is read-only.
|
||||
@@ -508,6 +524,7 @@ class MemoryFileHandle extends VirtualFileHandle {
|
||||
if (this.#entry?.isDynamic && this.#entry.isDynamic()) {
|
||||
return this.#entry.getContentSync();
|
||||
}
|
||||
this.#sync();
|
||||
return this.#content.subarray(0, this.#size);
|
||||
}
|
||||
|
||||
@@ -581,6 +598,7 @@ class MemoryFileHandle extends VirtualFileHandle {
|
||||
writeSync(buffer, offset, length, position) {
|
||||
this.#checkClosed('write');
|
||||
this.#checkWritable('write');
|
||||
this.#sync();
|
||||
|
||||
// In append mode, always write at the end
|
||||
const useCurrentPosition = isCurrentPosition(position);
|
||||
@@ -612,6 +630,7 @@ class MemoryFileHandle extends VirtualFileHandle {
|
||||
this.#entry.content = this.#content.subarray(0, this.#size);
|
||||
this.#entry.mtime = now;
|
||||
this.#entry.ctime = now;
|
||||
this.#published = this.#entry.content;
|
||||
}
|
||||
|
||||
// Update position if not using explicit position
|
||||
@@ -702,6 +721,7 @@ class MemoryFileHandle extends VirtualFileHandle {
|
||||
statSync(options) {
|
||||
this.#checkClosed('fstat');
|
||||
if (this.#getStats) {
|
||||
this.#sync();
|
||||
return this.#getStats(this.#size);
|
||||
}
|
||||
throw new ERR_INVALID_STATE('stats not available');
|
||||
@@ -750,6 +770,7 @@ class MemoryFileHandle extends VirtualFileHandle {
|
||||
truncateSync(len = 0) {
|
||||
this.#checkClosed('ftruncate');
|
||||
this.#checkWritable('ftruncate');
|
||||
this.#sync();
|
||||
|
||||
if (len < this.#size) {
|
||||
// Zero out truncated region to avoid stale data
|
||||
@@ -773,6 +794,7 @@ class MemoryFileHandle extends VirtualFileHandle {
|
||||
this.#entry.content = this.#content.subarray(0, this.#size);
|
||||
this.#entry.mtime = now;
|
||||
this.#entry.ctime = now;
|
||||
this.#published = this.#entry.content;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
// Flags: --disable-warning=ExperimentalWarning
|
||||
'use strict';
|
||||
|
||||
// MemoryProvider: an open handle is a live view of the file, not a snapshot
|
||||
// taken at open time. Writes made through the provider or through another
|
||||
// handle must be visible to handles that are already open, as they are on a
|
||||
// real filesystem. Refs: https://github.com/nodejs/node/issues/66355
|
||||
const common = require('../common');
|
||||
const assert = require('assert');
|
||||
const { create } = require('node:vfs');
|
||||
|
||||
// Content replaced through the provider is seen by an open read handle.
|
||||
{
|
||||
const { provider: p } = create();
|
||||
p.writeFileSync('/f', 'AAAA');
|
||||
const h = p.openSync('/f', 'r');
|
||||
p.writeFileSync('/f', '0123456789');
|
||||
|
||||
const b = Buffer.alloc(10);
|
||||
assert.strictEqual(h.readSync(b, 0, 10, 0), 10);
|
||||
assert.strictEqual(b.toString(), '0123456789');
|
||||
assert.strictEqual(h.readFileSync('utf8'), '0123456789');
|
||||
assert.strictEqual(h.statSync().size, 10);
|
||||
assert.strictEqual(p.statSync('/f').size, 10);
|
||||
h.readFile('utf8').then(common.mustCall((s) => {
|
||||
assert.strictEqual(s, '0123456789');
|
||||
h.closeSync();
|
||||
}));
|
||||
}
|
||||
|
||||
// A shrinking rewrite is seen too: reads stop at the new end.
|
||||
{
|
||||
const { provider: p } = create();
|
||||
p.writeFileSync('/f', '0123456789');
|
||||
const h = p.openSync('/f', 'r');
|
||||
p.writeFileSync('/f', 'AB');
|
||||
|
||||
const b = Buffer.alloc(10);
|
||||
assert.strictEqual(h.readSync(b, 0, 10, 0), 2);
|
||||
assert.strictEqual(b.subarray(0, 2).toString(), 'AB');
|
||||
assert.strictEqual(h.readFileSync('utf8'), 'AB');
|
||||
assert.strictEqual(h.statSync().size, 2);
|
||||
h.closeSync();
|
||||
}
|
||||
|
||||
// Two writable handles on the same file do not clobber each other.
|
||||
{
|
||||
const { provider: p } = create();
|
||||
p.writeFileSync('/f', 'AAAA');
|
||||
const h1 = p.openSync('/f', 'r+');
|
||||
const h2 = p.openSync('/f', 'r+');
|
||||
|
||||
h1.writeSync(Buffer.from('XY'), 0, 2, 0);
|
||||
h2.writeSync(Buffer.from('Z'), 0, 1, 3);
|
||||
assert.strictEqual(p.readFileSync('/f', 'utf8'), 'XYAZ');
|
||||
assert.strictEqual(h1.readFileSync('utf8'), 'XYAZ');
|
||||
|
||||
// Growing through one handle is visible to the other.
|
||||
h1.writeSync(Buffer.from('0123456789'), 0, 10, 4);
|
||||
assert.strictEqual(h2.statSync().size, 14);
|
||||
assert.strictEqual(h2.readFileSync('utf8'), 'XYAZ0123456789');
|
||||
|
||||
// Truncating through one handle is visible to the other.
|
||||
h2.truncateSync(3);
|
||||
assert.strictEqual(h1.statSync().size, 3);
|
||||
assert.strictEqual(h1.readFileSync('utf8'), 'XYA');
|
||||
|
||||
h1.closeSync();
|
||||
h2.closeSync();
|
||||
}
|
||||
|
||||
// A write through a handle opened before the file was replaced lands in the
|
||||
// current content instead of resurrecting the old bytes.
|
||||
{
|
||||
const { provider: p } = create();
|
||||
p.writeFileSync('/f', 'AAAA');
|
||||
const h = p.openSync('/f', 'r+');
|
||||
p.writeFileSync('/f', 'BB');
|
||||
|
||||
h.writeSync(Buffer.from('C'), 0, 1, 3);
|
||||
assert.deepStrictEqual(p.readFileSync('/f'), Buffer.from('BB\0C', 'latin1'));
|
||||
h.closeSync();
|
||||
}
|
||||
|
||||
// An append handle opened earlier appends after the current end.
|
||||
{
|
||||
const { provider: p } = create();
|
||||
p.writeFileSync('/f', 'AAAA');
|
||||
const h = p.openSync('/f', 'a');
|
||||
p.writeFileSync('/f', 'B');
|
||||
|
||||
h.writeSync(Buffer.from('C'), 0, 1, null);
|
||||
assert.strictEqual(p.readFileSync('/f', 'utf8'), 'BC');
|
||||
h.closeSync();
|
||||
}
|
||||
Reference in new issue
Block a user