src: fix fatal errors when a current isolate not exist · nodejs/node@99493b0 · GitHub

nodejs / node Public

• Notifications You must be signed in to change notification settings
• Fork 36.6k
• Star 120k

• Code
• Issues 635
• Pull requests 751
• Actions
• Projects
• Security and quality
• Insights

Additional navigation options

Commit 99493b0

Browse filesBrowse the repository at this point in the historyBrowse files

legendecas

authored and

targos

committed

src: fix fatal errors when a current isolate not exist

napi_fatal_error and node watchdog trigger fatal error but rather running on a thread that hold no current isolate. PR-URL: #38624 Backport-PR-URL: #39515 Reviewed-By: Michael Dawson <midawson@redhat.com>

1 parent a74032a commit 99493b0
Copy full SHA for 99493b0

5 files changed

+49Lines changed: 49 additions & 0 deletions

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

• src
  • node_errors.cc
• test/node-api/test_fatal
  • test.js
  • test2.js
  • test_fatal.c
  • test_threads.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

‎src/node_errors.cc‎

Copy file name to clipboardExpand all lines: src/node_errors.cc

+6Lines changed: 6 additions & 0 deletions

Original file line number	Diff line number	Diff line change
@@ -424,6 +424,12 @@ void OnFatalError(const char* location, const char* message) {
424	424	}
425	425
426	426	Isolate* isolate = Isolate::GetCurrent();
	427	+ // TODO(legendecas): investigate failures on triggering node-report with
	428	+ // nullptr isolates.
	429	+ if (isolate == nullptr) {
	430	+ fflush(stderr);
	431	+ ABORT();
	432	+ }
427	433	Environment* env = Environment::GetCurrent(isolate);
428	434	bool report_on_fatalerror;
429	435	{

Collapse file

‎test/node-api/test_fatal/test.js‎

Copy file name to clipboardExpand all lines: test/node-api/test_fatal/test.js

+1Lines changed: 1 addition & 0 deletions

Original file line number	Diff line number	Diff line change
@@ -16,3 +16,4 @@ const p = child_process.spawnSync(
16	16	assert.ifError(p.error);
17	17	assert.ok(p.stderr.toString().includes(
18	18	'FATAL ERROR: test_fatal::Test fatal message'));
	19	+ assert.ok(p.status === 134 || p.signal === 'SIGABRT');

Collapse file

‎test/node-api/test_fatal/test2.js‎

Copy file name to clipboardExpand all lines: test/node-api/test_fatal/test2.js

+1Lines changed: 1 addition & 0 deletions

Original file line number	Diff line number	Diff line change
@@ -16,3 +16,4 @@ const p = child_process.spawnSync(
16	16	assert.ifError(p.error);
17	17	assert.ok(p.stderr.toString().includes(
18	18	'FATAL ERROR: test_fatal::Test fatal message'));
	19	+ assert.ok(p.status === 134 || p.signal === 'SIGABRT');

Collapse file

‎test/node-api/test_fatal/test_fatal.c‎

Copy file name to clipboardExpand all lines: test/node-api/test_fatal/test_fatal.c

+21Lines changed: 21 additions & 0 deletions

Original file line number	Diff line number	Diff line change
@@ -1,12 +1,32 @@
	1	+ // For the purpose of this test we use libuv's threading library. When deciding
	2	+ // on a threading library for a new project it bears remembering that in the
	3	+ // future libuv may introduce API changes which may render it non-ABI-stable,
	4	+ // which, in turn, may affect the ABI stability of the project despite its use
	5	+ // of N-API.
	6	+ #include <uv.h>
1	7	#include <node_api.h>
2	8	#include "../../js-native-api/common.h"
3	9
	10	+ static uv_thread_t uv_thread;
	11	+
	12	+ static void work_thread(void* data) {
	13	+ napi_fatal_error("work_thread", NAPI_AUTO_LENGTH,
	14	+ "foobar", NAPI_AUTO_LENGTH);
	15	+ }
	16	+
4	17	static napi_value Test(napi_env env, napi_callback_info info) {
5	18	napi_fatal_error("test_fatal::Test", NAPI_AUTO_LENGTH,
6	19	"fatal message", NAPI_AUTO_LENGTH);
7	20	return NULL;
8	21	}
9	22
	23	+ static napi_value TestThread(napi_env env, napi_callback_info info) {
	24	+ NAPI_ASSERT(env,
	25	+ (uv_thread_create(&uv_thread, work_thread, NULL) == 0),
	26	+ "Thread creation");
	27	+ return NULL;
	28	+ }
	29	+
10	30	static napi_value TestStringLength(napi_env env, napi_callback_info info) {
11	31	napi_fatal_error("test_fatal::TestStringLength", 16, "fatal message", 13);
12	32	return NULL;
@@ -16,6 +36,7 @@ static napi_value Init(napi_env env, napi_value exports) {
16	36	napi_property_descriptor properties[] = {
17	37	DECLARE_NAPI_PROPERTY("Test", Test),
18	38	DECLARE_NAPI_PROPERTY("TestStringLength", TestStringLength),
	39	+ DECLARE_NAPI_PROPERTY("TestThread", TestThread),
19	40	};
20	41
21	42	NAPI_CALL(env, napi_define_properties(

Collapse file

‎test/node-api/test_fatal/test_threads.js‎

Copy file name to clipboard

+20Lines changed: 20 additions & 0 deletions

Original file line number	Diff line number	Diff line change
@@ -0,0 +1,20 @@
	1	+ 'use strict';
	2	+ const common = require('../../common');
	3	+ const assert = require('assert');
	4	+ const child_process = require('child_process');
	5	+ const test_fatal = require(`./build/${common.buildType}/test_fatal`);
	6	+
	7	+ // Test in a child process because the test code will trigger a fatal error
	8	+ // that crashes the process.
	9	+ if (process.argv[2] === 'child') {
	10	+ test_fatal.TestThread();
	11	+ // Busy loop to allow the work thread to abort.
	12	+ while (true) {}
	13	+ }
	14	+
	15	+ const p = child_process.spawnSync(
	16	+ process.execPath, [ __filename, 'child' ]);
	17	+ assert.ifError(p.error);
	18	+ assert.ok(p.stderr.toString().includes(
	19	+ 'FATAL ERROR: work_thread foobar'));
	20	+ assert.ok(p.status === 134 || p.signal === 'SIGABRT');

0 commit comments

Comments

0 (0)