Change in fs.statSync behavior between 7.1.0 and 7.2.0

[sharkinsspatial]

• Version: 7.1.0 & 7.2.0
• Platform: OSX 10.9.5
• Subsystem: fs

I noted a change in behavior between fs.statSync when migrating between 7.1.0 and 7.2.0. With
7.1.0 the following code reports a file size of 0.

var fs = require('fs');

var buffer = new Buffer(16);
buffer.fill(0);

fd = fs.openSync('test', 'a');
fs.writeSync(fd, buffer);
fs.fsyncSync(fd);
fs.closeSync(fd);

var stats = fs.statSync('./test');
console.log(stats['size']);
fs.unlinkSync('test');

While using 7.2.0 it reports a file size of 16.

Interestingly using appendFileSync rather than opening file handles is not affected by version differences. The Changelog notes a libuv upgrade between the two versions but I'm unsure what could produce this change.

[Fishrock123]

cc @nodejs/fs @saghul

[trevnorris]

Revised test:

'use strict';

const fs = require('fs');
const buffer = Buffer.alloc(16);
const FILENAME = '/tmp/test';

fs.writeFileSync(FILENAME, 'wx');

const fd = fs.openSync(FILENAME, 'a');
fs.writeSync(fd, buffer);
fs.fsyncSync(fd);
fs.closeSync(fd);

var stats = fs.statSync(FILENAME);
console.log(stats.size);
//fs.unlinkSync(FILENAME);

xxd output from node v6.9.1:

00000000: 0000 0000 0000 0000 0000 0000 0000 0000  ................

from node v7.2.1-pre:

00000000: 0000 0000 0000 0000 0000 0000 0000 0000  ................
00000010: 0000 0000 0000 0000 0000 0000 0000 0000  ................

If the script is changed to use

const buffer = Buffer.alloc(16, 'abc');

output from v6.9.1 is

00000000: 6162 6361 6263 6162 6361 6263 6162 6361  abcabcabcabcabca

whereas v7.2.1-pre is

00000000: 6162 6361 6263 6162 6361 6263 6162 6361  abcabcabcabcabca
00000010: 6162 6361 6263 6162 6361 6263 6162 6361  abcabcabcabcabca

This leads me to believe the file contents are written out twice.

EDIT: This was running on Linux 4.4.0 x64 using ext4

EDIT 2: Accidentally wrote buffer the first time (which is how it's done in the OP; is that correct?), instead of an empty buffer. So just the behavior is different (possibly something related to #9699)

[trevnorris]

Okay, try this again. Slightly simplified test that demonstrates the same as in the OP:

'use strict';
const fs = require('fs');
const FILENAME = '/tmp/test';

fs.writeFileSync(FILENAME, Buffer.alloc(0), { flag: 'wx' });

const fd = fs.openSync(FILENAME, 'a');
fs.writeSync(fd, Buffer.alloc(16, 'abc'));

console.log(fs.statSync(FILENAME).size);
fs.closeSync(fd);

On v6.9.1 output is 0 and on v7.2.1-pre output is 16.

[trevnorris]

@sharkinsspatial In short, this actually don't have to do w/ statSync(). This has to do with the way the file itself is being written out.

[bnoordhuis]

It's caused by #7856, reverting 8a9c45a reverts to the v7.1.0 behavior. The libuv upgrade does not contain any changes that could cause such a change in behavior.

[papandreou]

I tried adding a console.log of the arguments handed to binding.writeBuffer here when running @trevnorris' repro, and as expected there's a difference between 7.1 and 7.2:

With 7.1.0:

binding.writeBuffer(9, <Buffer 61 62 63 61 62 63 61 62 63 61 62 63 61 62 63 61>, undefined, undefined, null);

With 7.2.0:

binding.writeBuffer(9, <Buffer 61 62 63 61 62 63 61 62 63 61 62 63 61 62 63 61>, 0, 16, null);

It looks correct to me, though -- but maybe it's surfacing a problem with binding.writeBuffer because offset and length are passed explicitly?

[MylesBorins]

@nodejs/ctc has everyone seen this? Is there any action to be done?

[papandreou]

It seems like the difference in behavior kicks in here. With 7.1.0, len ended up being 0 here:

uv_buf_t uvbuf = uv_buf_init(const_cast<char*>(buf), len);

... but in 7.2.0 length is being defaulted to 16 in fs.writeSync because the buffer is 16 bytes long (and no offset is given), and thus len ends up being 16 here too.

The libuv docs don't say explicitly what passing 0 means, but a bit of logging (printf("Got uv_buf_t of length %u\n", uvbuf.len);) reveals that it allocates a uv_buf_t of 0 bytes and then writes that. And that's clearly not the intention.

It seems like the behavior of 7.2.0 is the correct one. It's just that fs.writeSync(fd, Buffer.alloc(16, 'abc')); used to end up writing 0 bytes because no reasonable default values were provided for the offset and length parameters.

@sharkinsspatial, what exactly did you expect the snippet to do? Did you just observe that the behavior changed?

[sharkinsspatial]

Sorry for the delay in response (holiday vacation). For me the 7.2.0 is the correct expected behavior. I just ran into an issue when I deployed to a pre-7.2.0 environment and the behavior changed. This discussion explains it clearly for me.

[papandreou]

@sharkinsspatial Okay, thanks for clarifying :)

I believe we can close this issue.

[thefourtheye]

I feel that this has to be documented somewhere.

[papandreou]

@thefourtheye, which part exactly? The way I see it, the implementation of fs.writeSync now conforms to what the docs have said all along. See #7856 and the linked issues.

[gibfahn]

It sounds like this issue can be closed (reopen if you disagree).