Add wrappers for node_api_nogc_env and node_api_nogc_finalize

[KevinEady]

Since nodejs/node#50060 is now in core, we need to add support for these types.

Looking at the Node-API docs, we can find which node-addon-api methods on Napi::Env should be split between normal and nogc variants. The normal variant can use nogc methods, but not the other way around:

• nogc:
  • AddCleanupHook
  • GetInstanceData
  • SetInstanceData
  • GetModuleFileName
• normal:
  • Global
  • Undefined
  • Null
  • IsExceptionPending
  • GetAndClearPendingException
  • RunScript

Additionally, all JavaScript-using paradigms (creating new Values, calling functions, ...) should use the normal variant, and have a compile-time error if a nogc variant is supplied.

[KevinEady]

Currently, the four nogc methods call NAPI_THROW_IF_FAILED_VOID, creating a new Napi::Error. This won't work in nogc-land since it's creating a new Value. I would like to discuss this on the 24 May Node-API call.

[KevinEady]

Also struggling a bit with figuring out how to make napi[-inl].h smart enough to know when node_api_nogc_env is available for use. For example, we cannot use these new types if compiling with headers for node v18.17.1, and I don't think we want to break compilation on non-most-recent-Node.js builds.

[KevinEady]

Discussed in 24 May Node-API meeting:

• Check under which circumstances the nogc functions could fail; if they are "reasonable" (eg. invalid parameters passed), we could just some fatal exception instead
• Use the POST FINALIZER definition to guard against nogc types

[KevinEady]

Another issue that i'm struggling with regarding ObjectWrap. The destructor uses Reference<Napi::Object>::Value() which calls napi_get_reference_value, and this method cannot be used inside GC:

node-addon-api/napi-inl.h

Lines 4438 to 4450 in 7c79c33

	template <typename T>
	inline ObjectWrap<T>::~ObjectWrap() {
	// If the JS object still exists at this point, remove the finalizer added
	// through `napi_wrap()`.
	if (!IsEmpty()) {
	Object object = Value();
	// It is not valid to call `napi_remove_wrap()` with an empty `object`.
	// This happens e.g. during garbage collection.
	if (!object.IsEmpty() && _construction_failed) {
	napi_remove_wrap(Env(), object, nullptr);
	}
	}
	}

node-addon-api/napi-inl.h

Lines 3265 to 3275 in 7c79c33

	template <typename T>
	inline T Reference<T>::Value() const {
	if (_ref == nullptr) {
	return T(_env, nullptr);
	}
	napi_value value;
	napi_status status = napi_get_reference_value(_env, _ref, &value);
	NAPI_THROW_IF_FAILED(_env, status, T());
	return T(_env, value);
	}

Thoughts on how to proceed in this use case...? Does this napi_remove_wrap need to be moved into a post-finalizer block, scheduled/added at the ObjectWrap instance construction?

cc: @legendecas @gabrielschulhof @vmoroz

[KevinEady]

Additionally, in ObjectWrap construction, the napi_wrap gets passed ObjectWrap<T>::FinalizeCallback, which in turn creates some handle scopes, which cannot be done in finalization.

node-addon-api/napi-inl.h

Line 4431 in 7c79c33

status = napi_wrap(env, wrapper, instance, FinalizeCallback, nullptr, &ref);

Not sure how to proceed on this either.

node-addon-api/napi-inl.h

Lines 4878 to 4886 in 7c79c33

	template <typename T>
	inline void ObjectWrap<T>::FinalizeCallback(napi_env env,
	void* data,
	void* /*hint*/) {
	HandleScope scope(env);
	T* instance = static_cast<T*>(data);
	instance->Finalize(Napi::Env(env));
	delete instance;
	}

[KevinEady]

So continuing to dig a bit, mostly keeping this as notes... The combination of the destructor ~ObjectWrap and the FinalizeCallback is supposed to behave in this manner:

• When the C++ object is deleted, the destructor removes the wrap if the reference holds a value.
• When the JS object is garbage collected, the user's custom virtual void Finalize(Env) runs.

So, there are two scenarios we need to cover:

1. The C++ object is deleted prior to GC. Eg: Creating an ObjectWrap'd instance inside a native C++ method on the stack that is never returned to Node, causing the deletion when the object goes out of scope.
2. The JS object is GC'd prior to destruction. Eg: Creating an ObjectWrap'd instance and returning it back to Node for ownership.

[legendecas]

Seems like this is an exploit of the behavior of napi_get_reference_value. napi_get_reference_value will fail if it is called in finalizers without opening a handle scope. However, in a finalizer, the reference was reset and napi_get_reference_value will simply return a nullptr (ref).

[KevinEady]

napi_get_reference_value will fail if it is called in finalizers without opening a handle scope.

From my understanding, this is intended behavior...? Since the function takes a napi_env (vs. the nogc counterpart), I expect that it requires some sort of JavaScript engine integration/usage.

Hopefully we can discuss this in today's meeting.

[github-actions]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.