node
node copied to clipboard
NAPI: Add Symbols invisible to getOwnPropertySymbols
What is the problem this feature will solve?
This is in the context of implementing dawn.node which is a Node module exposing the WebGPU API by translating JS calls to calls to Dawn, the library that Chromium uses to implement WebGPU.
The GPUDevice inherits from EventTarget
so we need to implement a reasonable version of EventTarget
in dawn.node. This requires keeping a list of callbacks to call them in dispatchEvent
. Because Napi::Value
are only valid during the scope of a call by default, the C++ implementation must keep a Napi::Reference
to them so that the callback values stay valid between the call to addEventListener
and dispatchEvent
. However this could lead to reference cycles that prevent objects from being collected: the GPUDevice
owns a CppEventTargetImplementation
that contains Napi::Reference<Napi::Function>
that's a closure referencing the GPUDevice
. In this case the GPUDevice
can never be collected.
What is the feature you are proposing to solve the problem?
An alternative is to hide the storage of the EventTarget
values in the JS object graph by storing the data in an object keyed with a symbol only known by the C++ code. Essentiallydevice[SymbolKnownByCpp] = {eventTargetStuff}
. However this symbol would still be discoverable with getOwnPropertySymbols
so the encapsulation is not perfect (even with enumerable: false
).
Could we have a way to make symbol properties hidden from getOwnPropertySymbols
? Looking at the v8 and SpiderMonkey headers, both engines don't seem to have support for this, but maybe it could be added?
What alternatives have you considered?
I've tried keeping a Napi::Reference
in my code, but this will lead to leaks.
Another possibility would be to add a tracing API to node that integrates with the marking phase of GCs, similar to what's in Blink with the Trace
methods, but this seems like a MUCH harder problem and very engine dependent.
Looking at the v8 and SpiderMonkey headers, both engines don't seem to have support for this, but maybe it could be added?
Node.js core has used v8::Private
to hide such symbols. IIUC, it doesn't seem to exist in Node-APIs since it isn't a JavaScript feature.
/cc @nodejs/node-api Can we allow this kind of feature on Node APIs? It seems like a use case worth considering.
Is support for JS engines other than V8 still a design constraint for n-api? Because I don't think other engines have a similar feature.
Is support for JS engines other than V8 still a design constraint for n-api? Because I don't think other engines have a similar feature.
This is still a documented constraint that we should stick to when adding new node-api functions. These rules allow us to avoid introducing node-api that is vulnerable to specific JS engine implementation-defined features without ABI guarantees. Notably, we have node-api implementations on other JS engines available too:
- https://github.com/oven-sh/bun#node-api-napi
- https://github.com/jerryscript-project/iotjs/tree/master/src/napi
- More at https://github.com/nodejs/abi-stable-node/blob/doc/node-api-engine-bindings.md#node-api-bindings-for-other-runtimes.
We discussed this in the 9 Dec 2022 Node-API meeting. @vmoroz will take a look at the possibility of using private symbols in Hermes engine so we can get a better understanding of support of this feature in other JavaScript engines.
@Kangz If you want to have some fun in the short term, and maybe validate that private symbols as a concept will work for you, you can do something like this (not tested but should be more or less the right path i think...)
Napi::Value MakeSpookySymbol(Napi::Env env) {
Local<Private> p = Private::New(Isolate::GetCurrent()); // i think you can extract isolate from `env` as well but i don't feel like digging through the internals to remember how this works
Local<Value> sym = *reinterpret_cast<Local<Value>*>(&p);
napi_value n = reinterpret_cast<napi_value>(*sym);
return Napi::Value(env, n);
}
Also, you could consider using WeakMap, which has identical semantics to using this private symbol api, without relying on engine weirdness.
Thanks all for the discussion!and progress so far, this is much more than I expected :)
As for the Hermes JavaScript engine, they recently added a feature to associate native state with an Object. Comparing with V8, it means that Hermes has only a single predefined private property.
@Kangz, would it be possible to use std::weak_ptr to break the reference loop? The std::weak_ptr is often used inside of the lambda capture. The napi_ref can also be used as a weak reference - it becomes a weak reference when the ref count is zero.
This is still a documented constraint that we should stick to when adding new node-api functions. These rules allow us to avoid introducing node-api that is vulnerable to specific JS engine implementation-defined features without ABI guarantees. Notably, we have node-api implementations on other JS engines available too:
Speaking for bun, it is straightforward to add symbols invisible to getOwnPropertySymbols
Could we have a way to make symbol properties hidden from getOwnPropertySymbols? Looking at the v8 and SpiderMonkey headers, both engines don't seem to have support for this, but maybe it could be added?
The "common" way would be to use a WeakMap
to hold the references instead of a symbol on the object.
So instead of eventTarget[kBackingStorage]
you would have backingStorageMap.get(eventTarget)
.
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.
I think this can be solved with ObjectWrap. The Napi::ObjectWrap subclass contains a linked list of Napi::FunctionReference
objects, each of which is a weak reference. A finalizer is added to each Napi::FunctionReference
such that it removes the linked list item from the list when the Napi::FunctionReference
is collected. That way, there is no reference loop, and no private properties are needed. Behind the scenes, ObjectWrap is implemented using a private property anyway, so this would piggy-back on that.
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.
There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.
For more information on how the project manages feature requests, please consult the feature request management document.