Skip to content

Commit 026ae18

Browse files
committed
fs: fix flush regression from fast path
1 parent dbe45b7 commit 026ae18

File tree

3 files changed

+51
-15
lines changed

3 files changed

+51
-15
lines changed

lib/fs.js

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2381,27 +2381,37 @@ function writeFileSync(path, data, options) {
23812381
validateBoolean(flush, 'options.flush');
23822382

23832383
const flag = options.flag || 'w';
2384+
const isUserFd = isFd(path); // File descriptor ownership
23842385

23852386
// C++ fast path for string data and UTF8 encoding
2386-
if (typeof data === 'string' && (options.encoding === 'utf8' || options.encoding === 'utf-8')) {
2387+
if (
2388+
typeof data === 'string' &&
2389+
(options.encoding === 'utf8' || options.encoding === 'utf-8') &&
2390+
(isUserFd || !flush)
2391+
) {
23872392
if (!isInt32(path)) {
23882393
path = getValidatedPath(path);
23892394
}
23902395

2391-
return binding.writeFileUtf8(
2396+
binding.writeFileUtf8(
23922397
path,
23932398
data,
23942399
stringToFlags(flag),
23952400
parseFileMode(options.mode, 'mode', 0o666),
23962401
);
2402+
2403+
if (flush && isUserFd) {
2404+
fs.fsyncSync(path);
2405+
}
2406+
2407+
return;
23972408
}
23982409

23992410
if (!isArrayBufferView(data)) {
24002411
validateStringAfterArrayBufferView(data, 'data');
24012412
data = Buffer.from(data, options.encoding || 'utf8');
24022413
}
24032414

2404-
const isUserFd = isFd(path); // File descriptor ownership
24052415
const fd = isUserFd ? path : fs.openSync(path, flag, options.mode);
24062416

24072417
let offset = 0;

test/parallel/test-fs-append-file-flush.js

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const fs = require('node:fs');
66
const fsp = require('node:fs/promises');
77
const test = require('node:test');
88
const data = 'foo';
9+
const data2 = 'bar';
910
let cnt = 0;
1011

1112
function nextFile() {
@@ -25,15 +26,27 @@ test('synchronous version', async (t) => {
2526

2627
await t.test('performs flush', (t) => {
2728
const spy = t.mock.method(fs, 'fsyncSync');
29+
const checkCalls = (expected) => {
30+
const calls = spy.mock.calls;
31+
assert.strictEqual(calls.length, expected);
32+
if (expected === 0) return;
33+
assert.strictEqual(calls.at(-1).result, undefined);
34+
assert.strictEqual(calls.at(-1).error, undefined);
35+
assert.strictEqual(calls.at(-1).arguments.length, 1);
36+
assert.strictEqual(typeof calls.at(-1).arguments[0], 'number');
37+
};
38+
2839
const file = nextFile();
40+
41+
checkCalls(0);
2942
fs.appendFileSync(file, data, { flush: true });
30-
const calls = spy.mock.calls;
31-
assert.strictEqual(calls.length, 1);
32-
assert.strictEqual(calls[0].result, undefined);
33-
assert.strictEqual(calls[0].error, undefined);
34-
assert.strictEqual(calls[0].arguments.length, 1);
35-
assert.strictEqual(typeof calls[0].arguments[0], 'number');
43+
checkCalls(1);
3644
assert.strictEqual(fs.readFileSync(file, 'utf8'), data);
45+
46+
checkCalls(0);
47+
fs.appendFileSync(file, data2, { flush: true });
48+
checkCalls(1);
49+
assert.strictEqual(fs.readFileSync(file, 'utf8'), data + data2);
3750
});
3851

3952
await t.test('does not perform flush', (t) => {

test/parallel/test-fs-write-file-flush.js

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const fs = require('node:fs');
66
const fsp = require('node:fs/promises');
77
const test = require('node:test');
88
const data = 'foo';
9+
const data2 = 'bar';
910
let cnt = 0;
1011

1112
function nextFile() {
@@ -25,15 +26,27 @@ test('synchronous version', async (t) => {
2526

2627
await t.test('performs flush', (t) => {
2728
const spy = t.mock.method(fs, 'fsyncSync');
29+
const checkCalls = (expected) => {
30+
const calls = spy.mock.calls;
31+
assert.strictEqual(calls.length, expected);
32+
if (expected === 0) return;
33+
assert.strictEqual(calls.at(-1).result, undefined);
34+
assert.strictEqual(calls.at(-1).error, undefined);
35+
assert.strictEqual(calls.at(-1).arguments.length, 1);
36+
assert.strictEqual(typeof calls.at(-1).arguments[0], 'number');
37+
};
38+
2839
const file = nextFile();
40+
41+
checkCalls(0);
2942
fs.writeFileSync(file, data, { flush: true });
30-
const calls = spy.mock.calls;
31-
assert.strictEqual(calls.length, 1);
32-
assert.strictEqual(calls[0].result, undefined);
33-
assert.strictEqual(calls[0].error, undefined);
34-
assert.strictEqual(calls[0].arguments.length, 1);
35-
assert.strictEqual(typeof calls[0].arguments[0], 'number');
43+
checkCalls(1);
3644
assert.strictEqual(fs.readFileSync(file, 'utf8'), data);
45+
46+
checkCalls(1);
47+
fs.writeFileSync(file, data2, { flush: true, encoding: 'utf8' });
48+
checkCalls(2);
49+
assert.strictEqual(fs.readFileSync(file, 'utf8'), data2);
3750
});
3851

3952
await t.test('does not perform flush', (t) => {

0 commit comments

Comments
 (0)