Skip to content

Commit 8c00a80

Browse files
committed
fs: throw fs.fstat{Sync} errors in JS
PR-URL: #17914 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent da7804f commit 8c00a80

File tree

3 files changed

+29
-19
lines changed

3 files changed

+29
-19
lines changed

lib/fs.js

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -633,12 +633,11 @@ function readFileAfterClose(err) {
633633
}
634634

635635
function tryStatSync(fd, isUserFd) {
636-
var threw = true;
637-
try {
638-
binding.fstat(fd);
639-
threw = false;
640-
} finally {
641-
if (threw && !isUserFd) fs.closeSync(fd);
636+
const ctx = {};
637+
binding.fstat(fd, undefined, ctx);
638+
if (ctx.errno !== undefined && !isUserFd) {
639+
fs.closeSync(fd);
640+
throw new errors.uvException(ctx);
642641
}
643642
}
644643

@@ -1115,7 +1114,11 @@ fs.stat = function(path, callback) {
11151114

11161115
fs.fstatSync = function(fd) {
11171116
validateUint32(fd, 'fd');
1118-
binding.fstat(fd);
1117+
const ctx = { fd };
1118+
binding.fstat(fd, undefined, ctx);
1119+
if (ctx.errno !== undefined) {
1120+
throw new errors.uvException(ctx);
1121+
}
11191122
return statsFromValues();
11201123
};
11211124

src/node_file.cc

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -587,19 +587,24 @@ static void LStat(const FunctionCallbackInfo<Value>& args) {
587587

588588
static void FStat(const FunctionCallbackInfo<Value>& args) {
589589
Environment* env = Environment::GetCurrent(args);
590+
Local<Context> context = env->context();
590591

591592
CHECK(args[0]->IsInt32());
592593

593-
int fd = args[0]->Int32Value();
594+
int fd = static_cast<int>(args[0]->Int32Value(context).FromJust());
594595

595596
if (args[1]->IsObject()) { // fstat(fd, req)
596597
CHECK_EQ(args.Length(), 2);
597598
AsyncCall(env, args, "fstat", UTF8, AfterStat,
598599
uv_fs_fstat, fd);
599-
} else { // fstat(fd)
600-
SYNC_CALL(fstat, nullptr, fd)
601-
FillStatsArray(env->fs_stats_field_array(),
602-
static_cast<const uv_stat_t*>(SYNC_REQ.ptr));
600+
} else { // fstat(fd, undefined, ctx)
601+
CHECK_EQ(args.Length(), 3);
602+
fs_req_wrap req_wrap;
603+
int err = SyncCall(env, args[2], &req_wrap, "fstat", uv_fs_fstat, fd);
604+
if (err == 0) {
605+
FillStatsArray(env->fs_stats_field_array(),
606+
static_cast<const uv_stat_t*>(req_wrap.req.ptr));
607+
}
603608
}
604609
}
605610

test/parallel/test-fs-sync-fd-leak.js

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
require('../common');
2424
const assert = require('assert');
2525
const fs = require('fs');
26+
const uv = process.binding('uv');
2627

2728
// ensure that (read|write|append)FileSync() closes the file descriptor
2829
fs.openSync = function() {
@@ -39,29 +40,30 @@ fs.writeSync = function() {
3940
throw new Error('BAM');
4041
};
4142

42-
process.binding('fs').fstat = function() {
43-
throw new Error('BAM');
43+
process.binding('fs').fstat = function(fd, _, ctx) {
44+
ctx.errno = uv.UV_EBADF;
45+
ctx.syscall = 'fstat';
4446
};
4547

4648
let close_called = 0;
4749
ensureThrows(function() {
4850
fs.readFileSync('dummy');
49-
});
51+
}, 'EBADF: bad file descriptor, fstat');
5052
ensureThrows(function() {
5153
fs.writeFileSync('dummy', 'xxx');
52-
});
54+
}, 'BAM');
5355
ensureThrows(function() {
5456
fs.appendFileSync('dummy', 'xxx');
55-
});
57+
}, 'BAM');
5658

57-
function ensureThrows(cb) {
59+
function ensureThrows(cb, message) {
5860
let got_exception = false;
5961

6062
close_called = 0;
6163
try {
6264
cb();
6365
} catch (e) {
64-
assert.strictEqual(e.message, 'BAM');
66+
assert.strictEqual(e.message, message);
6567
got_exception = true;
6668
}
6769

0 commit comments

Comments
 (0)