Skip to content

Commit 917615e

Browse files
Gabriel Schulhoftargos
Gabriel Schulhof
authored andcommitted
n-api: back up env before async work finalize
We must back up the value of `_env` before calling the async work complete callback, because the complete callback may delete the instance in which `_env` is stored by calling `napi_delete_async_work`, and because we need to use it after the complete callback has completed. Fixes: #20966 PR-URL: #21129 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
1 parent 76d6709 commit 917615e

File tree

3 files changed

+66
-3
lines changed

3 files changed

+66
-3
lines changed

src/node_api.cc

+9-3
Original file line numberDiff line numberDiff line change
@@ -3393,13 +3393,19 @@ class Work : public node::AsyncResource, public node::ThreadPoolWork {
33933393

33943394
CallbackScope callback_scope(this);
33953395

3396-
NAPI_CALL_INTO_MODULE(_env,
3396+
// We have to back up the env here because the `NAPI_CALL_INTO_MODULE` macro
3397+
// makes use of it after the call into the module completes, but the module
3398+
// may have deallocated **this**, and along with it the place where _env is
3399+
// stored.
3400+
napi_env env = _env;
3401+
3402+
NAPI_CALL_INTO_MODULE(env,
33973403
_complete(_env, ConvertUVErrorCode(status), _data),
3398-
[this] (v8::Local<v8::Value> local_err) {
3404+
[env] (v8::Local<v8::Value> local_err) {
33993405
// If there was an unhandled exception in the complete callback,
34003406
// report it as a fatal exception. (There is no JavaScript on the
34013407
// callstack that can possibly handle it.)
3402-
v8impl::trigger_fatal_exception(_env, local_err);
3408+
v8impl::trigger_fatal_exception(env, local_err);
34033409
});
34043410

34053411
// Note: Don't access `work` after this point because it was
+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
'use strict';
2+
const common = require('../../common');
3+
const assert = require('assert');
4+
const test_async = require(`./build/${common.buildType}/test_async`);
5+
const iterations = 500;
6+
7+
let x = 0;
8+
const workDone = common.mustCall((status) => {
9+
assert.strictEqual(status, 0, 'Work completed successfully');
10+
if (++x < iterations) {
11+
setImmediate(() => test_async.DoRepeatedWork(workDone));
12+
}
13+
}, iterations);
14+
test_async.DoRepeatedWork(workDone);

test/addons-napi/test_async/test_async.cc

+43
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include <stdio.h>
12
#include <node_api.h>
23
#include "../common.h"
34

@@ -173,10 +174,52 @@ napi_value TestCancel(napi_env env, napi_callback_info info) {
173174
return nullptr;
174175
}
175176

177+
struct {
178+
napi_ref ref;
179+
napi_async_work work;
180+
} repeated_work_info = { nullptr, nullptr };
181+
182+
static void RepeatedWorkerThread(napi_env env, void* data) {}
183+
184+
static void RepeatedWorkComplete(napi_env env, napi_status status, void* data) {
185+
napi_value cb, js_status;
186+
NAPI_CALL_RETURN_VOID(env,
187+
napi_get_reference_value(env, repeated_work_info.ref, &cb));
188+
NAPI_CALL_RETURN_VOID(env,
189+
napi_delete_async_work(env, repeated_work_info.work));
190+
NAPI_CALL_RETURN_VOID(env,
191+
napi_delete_reference(env, repeated_work_info.ref));
192+
repeated_work_info.work = nullptr;
193+
repeated_work_info.ref = nullptr;
194+
NAPI_CALL_RETURN_VOID(env,
195+
napi_create_uint32(env, (uint32_t)status, &js_status));
196+
NAPI_CALL_RETURN_VOID(env,
197+
napi_call_function(env, cb, cb, 1, &js_status, nullptr));
198+
}
199+
200+
static napi_value DoRepeatedWork(napi_env env, napi_callback_info info) {
201+
size_t argc = 1;
202+
napi_value cb, name;
203+
NAPI_ASSERT(env, repeated_work_info.ref == nullptr,
204+
"Reference left over from previous work");
205+
NAPI_ASSERT(env, repeated_work_info.work == nullptr,
206+
"Work pointer left over from previous work");
207+
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &cb, nullptr, nullptr));
208+
NAPI_CALL(env, napi_create_reference(env, cb, 1, &repeated_work_info.ref));
209+
NAPI_CALL(env,
210+
napi_create_string_utf8(env, "Repeated Work", NAPI_AUTO_LENGTH, &name));
211+
NAPI_CALL(env,
212+
napi_create_async_work(env, nullptr, name, RepeatedWorkerThread,
213+
RepeatedWorkComplete, &repeated_work_info, &repeated_work_info.work));
214+
NAPI_CALL(env, napi_queue_async_work(env, repeated_work_info.work));
215+
return nullptr;
216+
}
217+
176218
napi_value Init(napi_env env, napi_value exports) {
177219
napi_property_descriptor properties[] = {
178220
DECLARE_NAPI_PROPERTY("Test", Test),
179221
DECLARE_NAPI_PROPERTY("TestCancel", TestCancel),
222+
DECLARE_NAPI_PROPERTY("DoRepeatedWork", DoRepeatedWork),
180223
};
181224

182225
NAPI_CALL(env, napi_define_properties(

0 commit comments

Comments
 (0)