napi_create_reference/napi_delete_reference seems causing crashing in a very special situation

[toddwong]

• Version: v10.13.0
• Platform: Windows 10 64bit
• Subsystem: Napi

The below code causes node crashing:

const addon = require('./path/to/addon.node');

setInterval(function () {
    for (let j = 0; j < 1000000; j++)
        new addon.Test();
    global.gc();
    console.log("-----");
});

The addon code:

#include <node_api.h>

void finalize(napi_env env, void* finialize_data, void* finalize_hint) {
	auto ptr = (napi_ref*)finialize_data;
	napi_delete_reference(env, *ptr);
	delete ptr;
}

napi_value initialize(napi_env env, napi_callback_info info) {
	napi_value that;
	napi_get_cb_info(env, info, 0, nullptr, &that, nullptr);

	napi_ref* ptr = new napi_ref();
	napi_wrap(env, that, ptr, finalize, nullptr, nullptr);
	napi_create_reference(env, that, 0, ptr);

	return that;
}

NAPI_MODULE_INIT()
{
	napi_value cls;
	napi_define_class(env, "Test", NAPI_AUTO_LENGTH, initialize, nullptr, 0, nullptr, &cls);
	napi_set_named_property(env, exports, "Test", cls);

	return exports;
}

But if change the initialize function slightly:

	napi_ref* ptr = new napi_ref();
	// napi_wrap(env, that, ptr, finalize, nullptr, nullptr);
	// napi_create_reference(env, that, 0, ptr);
	napi_wrap(env, that, ptr, finalize, nullptr, ptr);

it will work just fine.

Actually, the bugy code was written very accidentally, and of course should be simplified to the good one anyway. I file this issue just wanna make sure there is no other potential bug hidden behind this. Hope some can confirm this, thanks.

[joaocgreis]

cc @nodejs/n-api

[toddwong]

Fixed by #24494

[gabrielschulhof]

Fixed in d45e303 (#24494). @toddwong please reopen this issue if you find that the crash persists after this fix!

[gabrielschulhof]

@toddwong NM 🙂 Should refresh before commenting 🙂

[toddwong]

😄