Skip to content

Commit 5dbc818

Browse files
fixup! fs: add mkstemp()
Signed-off-by: marcopiraccini <marco.piraccini@gmail.com>
1 parent c7570b3 commit 5dbc818

2 files changed

Lines changed: 48 additions & 10 deletions

File tree

‎src/node_file.cc‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,13 @@ void AfterOpenFileHandle(uv_fs_t* req) {
892892
}
893893
}
894894

895+
// Closes a file created by mkstemp() whose descriptor cannot be handed to JS.
896+
static void CloseMkstempFd(int fd) {
897+
uv_fs_t close_req;
898+
uv_fs_close(nullptr, &close_req, fd, nullptr);
899+
uv_fs_req_cleanup(&close_req);
900+
}
901+
895902
// Delivers the result of mkstemp(): [path, fd], or [path, FileHandle].
896903
static void AfterMkstempImpl(uv_fs_t* req, bool as_file_handle) {
897904
BaseObjectPtr<FSReqBase> req_wrap{FSReqBase::from_req(req)};
@@ -913,9 +920,10 @@ static void AfterMkstempImpl(uv_fs_t* req, bool as_file_handle) {
913920
after.Clear();
914921
return req_wrap->Reject(exception);
915922
}
916-
if (!after.Proceed()) return;
917-
918923
const int fd = static_cast<int>(req->result);
924+
// The environment is shutting down: nothing will receive the descriptor.
925+
if (!after.Proceed()) return CloseMkstempFd(fd);
926+
919927
Local<Value> path;
920928
Local<Value> error;
921929
{
@@ -928,17 +936,15 @@ static void AfterMkstempImpl(uv_fs_t* req, bool as_file_handle) {
928936
}
929937
}
930938
if (!error.IsEmpty()) {
931-
uv_fs_t close_req;
932-
uv_fs_close(nullptr, &close_req, fd, nullptr);
933-
uv_fs_req_cleanup(&close_req);
939+
CloseMkstempFd(fd);
934940
return req_wrap->Reject(error);
935941
}
936942

937943
Local<Value> file;
938944
if (as_file_handle) {
939945
FileHandle* handle =
940946
FileHandle::New(req_wrap->binding_data(), fd, {}, req->path);
941-
if (handle == nullptr) return;
947+
if (handle == nullptr) return CloseMkstempFd(fd);
942948
file = handle->object();
943949
} else {
944950
env->AddUnmanagedFd(fd);
@@ -4596,10 +4602,7 @@ static void Mkstemp(const FunctionCallbackInfo<Value>& args) {
45964602
Local<Value> path;
45974603
if (!StringBytes::Encode(isolate, req_wrap_sync.req.path, encoding)
45984604
.ToLocal(&path)) {
4599-
uv_fs_t close_req;
4600-
uv_fs_close(nullptr, &close_req, fd, nullptr);
4601-
uv_fs_req_cleanup(&close_req);
4602-
return;
4605+
return CloseMkstempFd(fd);
46034606
}
46044607
env->AddUnmanagedFd(fd);
46054608
Local<Value> result[] = {path, Integer::New(isolate, fd)};
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
'use strict';
2+
3+
// A file created by mkstemp() must be closed if the worker terminates before
4+
// the result can be delivered.
5+
6+
const common = require('../common');
7+
if (!common.isLinux) common.skip('counts descriptors through /proc/self/fd');
8+
9+
const assert = require('assert');
10+
const fs = require('fs');
11+
const { once } = require('events');
12+
const { Worker } = require('worker_threads');
13+
const tmpdir = require('../common/tmpdir');
14+
15+
tmpdir.refresh();
16+
17+
const countFds = () => fs.readdirSync('/proc/self/fd').length;
18+
19+
(async () => {
20+
const before = countFds();
21+
for (const method of ['fs.mkstemp', 'fs.promises.mkstemp']) {
22+
const worker = new Worker(`
23+
const fs = require('fs');
24+
const { parentPort } = require('worker_threads');
25+
for (let i = 0; i < 64; i++) {
26+
const result = ${method}(${JSON.stringify(tmpdir.resolve('terminate-'))}, () => {});
27+
result?.catch(() => {});
28+
}
29+
parentPort.postMessage('started');
30+
`, { eval: true });
31+
await once(worker, 'message');
32+
await worker.terminate();
33+
}
34+
assert.strictEqual(countFds(), before);
35+
})().then(common.mustCall());

0 commit comments

Comments
 (0)