Skip to content

Commit

Permalink
n-api: back up env before async work finalize
Browse files Browse the repository at this point in the history
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 <apapirovski@mac.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
Gabriel Schulhof authored and evanlucas committed Jun 12, 2018
1 parent 16ef09f commit 53f8563
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 3 deletions.
12 changes: 9 additions & 3 deletions src/node_api.cc
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions test/addons-napi/test_async/test-loop.js
@@ -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);
43 changes: 43 additions & 0 deletions test/addons-napi/test_async/test_async.cc
@@ -1,3 +1,4 @@
#include <stdio.h>
#include <node_api.h>
#include "../common.h"

Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 53f8563

Please sign in to comment.