Skip to content

Conversation

NathanWalker
Copy link
Contributor

This case surfaced when doing fast HMR changes with Vite. During debug/development, these are benign and just due to race condition on js objects outliving their native wrapper very briefly. This just allows for quality of life debugging with fast HMR changes. Standard assert behavior is unchanged for release builds.

The small adjustment on IndexedPropertySetterCallback is a small related issue but important bugfix.

Original code:
if (wrapper == nullptr && wrapper->Type() != WrapperType::ObjCObject) { return; }

This is problematic because:

It can dereference wrapper when it’s null (the RHS of && is evaluated when the LHS is true).
The logic there was wrong: it only returns when both are true. If wrapper is non-null but not an ObjCObject, it wouldn’t return and would proceed with an incompatible wrapper.
Fixed code: if (wrapper == nullptr || wrapper->Type() != WrapperType::ObjCObject) { return; }
This is the intended guard:

  • Short-circuits safely when wrapper is null.
  • Returns for any non-ObjCObject wrapper type.
  • It also matches the getter’s guard just above, which already uses the same pattern. So this change prevents a possible null deref and aligns setter/getter behavior. It doesn't break existing behavior and makes the setter correctly bail out in the same cases the getter does.

@NathanWalker NathanWalker added this to the 9.0 milestone Sep 30, 2025
@NathanWalker NathanWalker merged commit 42a5328 into main Sep 30, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant