GeezerVison is showing github.com — node-api: force env shutdown deferring behavior · nodejs/node@b5b1ad3
File treeExpand file treeCollapse file tree Open diff view settings test/js-native-api/test_reference_double_free
Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: src/js_native_api_v8.cc +9Lines changed: 9 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -270,8 +270,17 @@ class RefBase : protected Finalizer, RefTracker { | 270 | 270 |
| 271 | 271 | protected: | 272 | 272 | inline void Finalize(bool is_env_teardown = false) override { | | 273 | + // During environment teardown we have to convert a strong reference to | | 274 | + // a weak reference to force the deferring behavior if the user's finalizer | | 275 | + // happens to delete this reference so that the code in this function that | | 276 | + // follows the call to the user's finalizer may safely access variables from | | 277 | + // this instance. | | 278 | + if (is_env_teardown && RefCount() > 0) _refcount = 0; | | 279 | + | 273 | 280 | if (_finalize_callback != nullptr) { | 274 | 281 | _env->CallFinalizer(_finalize_callback, _finalize_data, _finalize_hint); | | 282 | + // This ensures that we never call the finalizer twice. | | 283 | + _finalize_callback = nullptr; | 275 | 284 | } | 276 | 285 |
| 277 | 286 | // this is safe because if a request to delete the reference | |
Collapse file Copy file name to clipboard +11Lines changed: 11 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +{ | | 2 | + "targets": [ | | 3 | + { | | 4 | + "target_name": "test_reference_double_free", | | 5 | + "sources": [ | | 6 | + "../entry_point.c", | | 7 | + "test_reference_double_free.c" | | 8 | + ] | | 9 | + } | | 10 | + ] | | 11 | +} |
Collapse file Copy file name to clipboard +11Lines changed: 11 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +'use strict'; | | 2 | + | | 3 | +// This test makes no assertions. It tests a fix without which it will crash | | 4 | +// with a double free. | | 5 | + | | 6 | +const { buildType } = require('../../common'); | | 7 | + | | 8 | +const addon = require(`./build/${buildType}/test_reference_double_free`); | | 9 | + | | 10 | +{ new addon.MyObject(true); } | | 11 | +{ new addon.MyObject(false); } |
Collapse file Copy file name to clipboard +55Lines changed: 55 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +#include <stdlib.h> | | 2 | +#include <js_native_api.h> | | 3 | +#include "../common.h" | | 4 | + | | 5 | +static size_t g_call_count = 0; | | 6 | + | | 7 | +static void Destructor(napi_env env, void* data, void* nothing) { | | 8 | + napi_ref* ref = data; | | 9 | + NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, *ref)); | | 10 | + free(ref); | | 11 | +} | | 12 | + | | 13 | +static void NoDeleteDestructor(napi_env env, void* data, void* hint) { | | 14 | + napi_ref* ref = data; | | 15 | + size_t* call_count = hint; | | 16 | + | | 17 | + // This destructor must be called exactly once. | | 18 | + if ((*call_count) > 0) abort(); | | 19 | + *call_count = ((*call_count) + 1); | | 20 | + free(ref); | | 21 | +} | | 22 | + | | 23 | +static napi_value New(napi_env env, napi_callback_info info) { | | 24 | + size_t argc = 1; | | 25 | + napi_value js_this, js_delete; | | 26 | + bool delete; | | 27 | + napi_ref* ref = malloc(sizeof(*ref)); | | 28 | + | | 29 | + NAPI_CALL(env, | | 30 | + napi_get_cb_info(env, info, &argc, &js_delete, &js_this, NULL)); | | 31 | + NAPI_CALL(env, napi_get_value_bool(env, js_delete, &delete)); | | 32 | + | | 33 | + if (delete) { | | 34 | + NAPI_CALL(env, | | 35 | + napi_wrap(env, js_this, ref, Destructor, NULL, ref)); | | 36 | + } else { | | 37 | + NAPI_CALL(env, | | 38 | + napi_wrap(env, js_this, ref, NoDeleteDestructor, &g_call_count, ref)); | | 39 | + } | | 40 | + NAPI_CALL(env, napi_reference_ref(env, *ref, NULL)); | | 41 | + | | 42 | + return js_this; | | 43 | +} | | 44 | + | | 45 | +EXTERN_C_START | | 46 | +napi_value Init(napi_env env, napi_value exports) { | | 47 | + napi_value myobj_ctor; | | 48 | + NAPI_CALL(env, | | 49 | + napi_define_class( | | 50 | + env, "MyObject", NAPI_AUTO_LENGTH, New, NULL, 0, NULL, &myobj_ctor)); | | 51 | + NAPI_CALL(env, | | 52 | + napi_set_named_property(env, exports, "MyObject", myobj_ctor)); | | 53 | + return exports; | | 54 | +} | | 55 | +EXTERN_C_END |
|