Fix weak reference creation

[vmoroz]

Summary

Fix Node-API weak reference creation for HostObject instances.

Issue Description

Recent hermes-windows releases trigger intermittent crashes in the RNW e2etest suite. A typical stack trace looks like this:

ntdll.dll!KiUserExceptionDispatch() Line 771    Unknown
[Inline Frame] hermes.dll!std::unique_ptr<hermes::vm::DecoratedObject::Decoration,std::default_delete<hermes::vm::DecoratedObject::Decoration>>::get() Line 3457    C++
[Inline Frame] hermes.dll!hermes::vm::DecoratedObject::getDecoration() Line 50    C++
[Inline Frame] hermes.dll!hermes::vm::HostObject::getProxy() Line 124    C++
[Inline Frame] hermes.dll!hermes::vm::HostObject::get(hermes::vm::SymbolID name) Line 112    C++
hermes.dll!hermes::vm::JSObject::getNamedWithReceiver_RJS(...) Line 1082    C++
[Inline Frame] hermes.dll!hermes::vm::JSObject::getNamed_RJS(...) Line 1931    C++
hermes.dll!hermes::node_api::NodeApiEnvironment::getNamedProperty(...) Line 3482    C++
hermes.dll!hermes::node_api::NodeApiEnvironment::getExternalPropertyValue(...) Line 3620    C++
[Inline Frame] hermes.dll!hermes::node_api::NodeApiEnvironment::addObjectFinalizer(...) Line 3647    C++
hermes.dll!hermes::node_api::NodeApiReference::convertToWeakRootStorage(...) Line 1969    C++
[Inline Frame] hermes.dll!hermes::node_api::NodeApiReference::NodeApiReference(...) Line 1835    C++
hermes.dll!hermes::node_api::NodeApiReference::create(...) Line 1739    C++
hermes.dll!napi_create_reference(...) Line 6274    C++

The underlying issues are:

• We attempt to create a weak reference to a HostObject instance.

• Weak reference creation adds an internal property to the HostObject so the NodeApiReference can receive finalization notifications.

• In failing cases, the HostObject’s internal proxy is missing.

• Writing to a null proxy causes the crash.

• The missing proxy might be due to GC or some other lifecycle quirk, but the weak-reference path has several problems regardless:

  • We must not assign internal properties on a HostObject.
  • We must not read internal properties from a HostObject.
  • NodeApiEnvironment::getExternalPropertyValue used getNamedProperty, which walks the prototype chain even though internal properties must be own properties.
  • For napi_ref objects owned by user code, the finalizer does nothing; adding a finalizer is unnecessary.
  • Only one of the NodeApiReference subclasses ensured the referenced value stayed alive long enough.

Solution

• Refactored NodeApiReference (and subclasses) when initialRefCount == 0, meaning we have to create a weak reference:

  • Added a virtual needsFinalizerTracking() predicate that reports whether the weak reference needs finalizer notifications. (In the failing scenario, it now returns false.)
  • Updated convertToWeakRootStorage() to honor needsFinalizerTracking().
  • Because the predicate is virtual, we moved the weak-storage setup out of the base constructor. Instead, each create() helper now calls a new initializeStorage() method after construction.
  • initializeStorage() also handles the logic that keeps the value alive, so all three reference types now share the same protection.

• Hardened NodeApiEnvironment::getExternalPropertyValue():

  • Reject HostObject and JSProxy instances.
  • Use vm::JSObject::getOwnNamedDescriptor to access the internal slot without traversing the prototype chain.

Testing

• Ran the RNW e2etest suite five consecutive times with the updated hermes.dll; no crashes observed.
• All Hermes unit tests pass.

Microsoft Reviewers: Open in CodeFlow