Skip to content

Commit 7754b1e

Browse files
ChALkeRaduh95
authored andcommitted
fs: fix flush regression from fast path
1 parent 40c625b commit 7754b1e

3 files changed

Lines changed: 51 additions & 15 deletions

File tree

lib/fs.js

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

23902390
const flag = options.flag || 'w';
2391+
const isUserFd = isFd(path); // File descriptor ownership
23912392

23922393
// C++ fast path for string data and UTF8 encoding
2393-
if (typeof data === 'string' && (options.encoding === 'utf8' || options.encoding === 'utf-8')) {
2394+
if (
2395+
typeof data === 'string' &&
2396+
(options.encoding === 'utf8' || options.encoding === 'utf-8') &&
2397+
!(flush && !isUserFd)
2398+
) {
23942399
if (!isInt32(path)) {
23952400
path = getValidatedPath(path);
23962401
}
23972402

2398-
return binding.writeFileUtf8(
2403+
binding.writeFileUtf8(
23992404
path,
24002405
data,
24012406
stringToFlags(flag),
24022407
parseFileMode(options.mode, 'mode', 0o666),
24032408
);
2409+
2410+
if (flush && isUserFd) {
2411+
fs.fsyncSync(path);
2412+
}
2413+
2414+
return;
24042415
}
24052416

24062417
if (!isArrayBufferView(data)) {
24072418
validateStringAfterArrayBufferView(data, 'data');
24082419
data = Buffer.from(data, options.encoding || 'utf8');
24092420
}
24102421

2411-
const isUserFd = isFd(path); // File descriptor ownership
24122422
const fd = isUserFd ? path : fs.openSync(path, flag, options.mode);
24132423

24142424
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(1);
47+
fs.appendFileSync(file, data2, { flush: true });
48+
checkCalls(2);
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)