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: https://github.com/nodejs/node/issues/20966
PR-URL: https://github.com/nodejs/node/pull/21129
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This commit is contained in:
Gabriel Schulhof 2018-06-04 19:20:54 -04:00
parent 1aa582a97c
commit 991f4060ad
3 changed files with 66 additions and 3 deletions

View File

@ -3393,13 +3393,19 @@ class Work : public node::AsyncResource, public node::ThreadPoolWork {
CallbackScope callback_scope(this);
NAPI_CALL_INTO_MODULE(_env,
// We have to back up the env here because the `NAPI_CALL_INTO_MODULE` macro
// makes use of it after the call into the module completes, but the module
// may have deallocated **this**, and along with it the place where _env is
// stored.
napi_env env = _env;
NAPI_CALL_INTO_MODULE(env,
_complete(_env, ConvertUVErrorCode(status), _data),
[this] (v8::Local<v8::Value> local_err) {
[env] (v8::Local<v8::Value> local_err) {
// If there was an unhandled exception in the complete callback,
// report it as a fatal exception. (There is no JavaScript on the
// callstack that can possibly handle it.)
v8impl::trigger_fatal_exception(_env, local_err);
v8impl::trigger_fatal_exception(env, local_err);
});
// Note: Don't access `work` after this point because it was

View File

@ -0,0 +1,14 @@
'use strict';
const common = require('../../common');
const assert = require('assert');
const test_async = require(`./build/${common.buildType}/test_async`);
const iterations = 500;
let x = 0;
const workDone = common.mustCall((status) => {
assert.strictEqual(status, 0, 'Work completed successfully');
if (++x < iterations) {
setImmediate(() => test_async.DoRepeatedWork(workDone));
}
}, iterations);
test_async.DoRepeatedWork(workDone);

View File

@ -1,3 +1,4 @@
#include <stdio.h>
#include <node_api.h>
#include "../common.h"
@ -173,10 +174,52 @@ napi_value TestCancel(napi_env env, napi_callback_info info) {
return nullptr;
}
struct {
napi_ref ref;
napi_async_work work;
} repeated_work_info = { nullptr, nullptr };
static void RepeatedWorkerThread(napi_env env, void* data) {}
static void RepeatedWorkComplete(napi_env env, napi_status status, void* data) {
napi_value cb, js_status;
NAPI_CALL_RETURN_VOID(env,
napi_get_reference_value(env, repeated_work_info.ref, &cb));
NAPI_CALL_RETURN_VOID(env,
napi_delete_async_work(env, repeated_work_info.work));
NAPI_CALL_RETURN_VOID(env,
napi_delete_reference(env, repeated_work_info.ref));
repeated_work_info.work = nullptr;
repeated_work_info.ref = nullptr;
NAPI_CALL_RETURN_VOID(env,
napi_create_uint32(env, (uint32_t)status, &js_status));
NAPI_CALL_RETURN_VOID(env,
napi_call_function(env, cb, cb, 1, &js_status, nullptr));
}
static napi_value DoRepeatedWork(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value cb, name;
NAPI_ASSERT(env, repeated_work_info.ref == nullptr,
"Reference left over from previous work");
NAPI_ASSERT(env, repeated_work_info.work == nullptr,
"Work pointer left over from previous work");
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &cb, nullptr, nullptr));
NAPI_CALL(env, napi_create_reference(env, cb, 1, &repeated_work_info.ref));
NAPI_CALL(env,
napi_create_string_utf8(env, "Repeated Work", NAPI_AUTO_LENGTH, &name));
NAPI_CALL(env,
napi_create_async_work(env, nullptr, name, RepeatedWorkerThread,
RepeatedWorkComplete, &repeated_work_info, &repeated_work_info.work));
NAPI_CALL(env, napi_queue_async_work(env, repeated_work_info.work));
return nullptr;
}
napi_value Init(napi_env env, napi_value exports) {
napi_property_descriptor properties[] = {
DECLARE_NAPI_PROPERTY("Test", Test),
DECLARE_NAPI_PROPERTY("TestCancel", TestCancel),
DECLARE_NAPI_PROPERTY("DoRepeatedWork", DoRepeatedWork),
};
NAPI_CALL(env, napi_define_properties(