FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: don't end fs promises on Isolate termination · nodejs/node@df50131 · GitHub

/ node Public

Commit df50131

Browse files
santigimeno
authored and
targos
committed
fs: don't end fs promises on Isolate termination
This is specially prevalent in the case of having in-progress FileHandle operations in a worker thread while the Worker is exiting. Fixes: #42829 PR-URL: #42910 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
1 parent 5c0a24d commit df50131

4 files changed

Lines changed: 106 additions & 2 deletions

File tree

‎src/node_file-inl.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,10 @@ FSReqPromise<AliasedBufferT>::New(BindingData* binding_data,
156156

157157
template <typename AliasedBufferT>
158158
FSReqPromise<AliasedBufferT>::~FSReqPromise() {
159-
// Validate that the promise was explicitly resolved or rejected.
160-
CHECK(finished_);
159+
// Validate that the promise was explicitly resolved or rejected but only if
160+
// the Isolate is not terminating because in this case the promise might have
161+
// not finished.
162+
if (!env()->is_stopping()) CHECK(finished_);
161163
}
162164

163165
template <typename AliasedBufferT>

‎src/node_file.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,7 @@ MaybeLocal<Promise> FileHandle::ClosePromise() {
377377
std::unique_ptr<CloseReq> close(CloseReq::from_req(req));
378378
CHECK_NOT_NULL(close);
379379
close->file_handle()->AfterClose();
380+
if (!close->env()->can_call_into_js()) return;
380381
Isolate* isolate = close->env()->isolate();
381382
if (req->result < 0) {
382383
HandleScope handle_scope(isolate);
@@ -650,6 +651,10 @@ void FSReqAfterScope::Reject(uv_fs_t* req) {
650651
}
651652

652653
bool FSReqAfterScope::Proceed() {
654+
if (!wrap_->env()->can_call_into_js()) {
655+
return false;
656+
}
657+
653658
if (req_->result < 0) {
654659
Reject(req_);
655660
return false;

‎test/parallel/test-worker-fshandles-error-on-termination.js‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const fs = require('fs/promises');
6+
const { scheduler } = require('timers/promises');
7+
const { parentPort, Worker } = require('worker_threads');
8+
9+
const MAX_ITERATIONS = 20;
10+
const MAX_THREADS = 10;
11+
12+
// Do not use isMainThread so that this test itself can be run inside a Worker.
13+
if (!process.env.HAS_STARTED_WORKER) {
14+
process.env.HAS_STARTED_WORKER = 1;
15+
16+
function spinWorker(iter) {
17+
const w = new Worker(__filename);
18+
w.on('message', common.mustCall((msg) => {
19+
assert.strictEqual(msg, 'terminate');
20+
w.terminate();
21+
}));
22+
23+
w.on('exit', common.mustCall(() => {
24+
if (iter < MAX_ITERATIONS)
25+
spinWorker(++iter);
26+
}));
27+
}
28+
29+
for (let i = 0; i < MAX_THREADS; i++) {
30+
spinWorker(0);
31+
}
32+
} else {
33+
async function open_nok() {
34+
await assert.rejects(
35+
fs.open('this file does not exist'),
36+
{
37+
code: 'ENOENT',
38+
syscall: 'open'
39+
}
40+
);
41+
await scheduler.yield();
42+
await open_nok();
43+
}
44+
45+
// These async function calls never return as they are meant to continually
46+
// open nonexistent files until the worker is terminated.
47+
open_nok();
48+
open_nok();
49+
50+
parentPort.postMessage('terminate');
51+
}

‎test/parallel/test-worker-fshandles-open-close-on-termination.js‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const fs = require('fs/promises');
6+
const { scheduler } = require('timers/promises');
7+
const { parentPort, Worker } = require('worker_threads');
8+
9+
const MAX_ITERATIONS = 20;
10+
const MAX_THREADS = 10;
11+
12+
// Do not use isMainThread so that this test itself can be run inside a Worker.
13+
if (!process.env.HAS_STARTED_WORKER) {
14+
process.env.HAS_STARTED_WORKER = 1;
15+
16+
function spinWorker(iter) {
17+
const w = new Worker(__filename);
18+
w.on('message', common.mustCall((msg) => {
19+
assert.strictEqual(msg, 'terminate');
20+
w.terminate();
21+
}));
22+
23+
w.on('exit', common.mustCall(() => {
24+
if (iter < MAX_ITERATIONS)
25+
spinWorker(++iter);
26+
}));
27+
}
28+
29+
for (let i = 0; i < MAX_THREADS; i++) {
30+
spinWorker(0);
31+
}
32+
} else {
33+
async function open_close() {
34+
const fh = await fs.open(__filename);
35+
await fh.close();
36+
await scheduler.yield();
37+
await open_close();
38+
}
39+
40+
// These async function calls never return as they are meant to continually
41+
// open and close files until the worker is terminated.
42+
open_close();
43+
open_close();
44+
45+
parentPort.postMessage('terminate');
46+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL