Repository navigation
fix(runtime): keep a JS function's block apart from its own wrapper - #504
Draft
edusperoni wants to merge 2 commits into
Draft
edusperoni wants to merge 2 commits into
edusperoni wants to merge 2 commits into
Conversation
A function wrapped in interop.FunctionReference and then marshalled as a block loses its FunctionReference state, so passing it as a C function pointer afterwards asserts. Marshalling it as a block after interop.FunctionReference also stops reusing the block it cached. Cover both orders, interop.handleof across them, a function pointer argument that is not a FunctionReference, collection of such functions, and a worker torn down while native code holds the block.
A JS function marshalled as a block cached its BlockWrapper in the same slot that interop.FunctionReference keeps its wrapper in, so each use evicted the other: the FunctionReferenceWrapper leaked and a later function pointer marshal asserted, or the block cache stopped hitting. The block cache now lives in its own private slot on the function. The JSBlock still owns its BlockWrapper and only clears that slot while it still holds that wrapper, so ObjectManager never sees a JS block's wrapper. interop.handleof reports a FunctionReference's trampoline once it has one, and the live block otherwise. A second interop.FunctionReference on the same function keeps the existing wrapper and trampoline. A function pointer argument that is not a pointer, native function pointer or FunctionReference throws instead of asserting. Fixes #503
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cause
A JS function marshalled as a block cached its
BlockWrapperin the per-object slottns::SetValue/tns::GetValueuses, and that is the same slotinterop.FunctionReferencekeeps itsFunctionReferenceWrapperin. Each use evicted the other:FunctionReferenceWrapperleaked, along with any trampoline it had cached. A later function pointer marshal found aBlockWrapperand hittns::Assert(false)inInterop::WriteValue.Fix
tns::{Set,Get,Delete}JSBlockWrapper, private keyjsBlock). The function's own wrapper slot is left alone.Interop::JSBlockowns itsBlockWrapperand always deletes it in dispose. It clears the cache slot only while that slot still holds its own wrapper, and still under the isolate pin and Locker. A JS block's wrapper never sits in the slotObjectManager::DisposeValuereads, so teardown and GC never touch it. Cache hits still go throughTryRetainJSBlock.interop.handleof(fn)returns a FunctionReference's trampoline once one exists. Otherwise it returns the live cached block, and otherwise it throws as before.interop.sizeofof a function with a cached block still reports pointer size.new interop.FunctionReference(fn)on the same function keeps the existing wrapper and trampoline instead of leaking them.Tests
FunctionReferenceBlockTests.js(new, committed first; on main it aborts on the assert) covers:interop.handleofreturning the same block throughout;interop.FunctionReference;The existing "JS block outliving its worker" specs still test what they describe: a block outliving, or released by, its worker's teardown while the function is FunctionReference-registered. Teardown now disposes the function's
FunctionReferenceWrapper, and the block's own dispose frees the block wrapper.blockTeardownReleaseWorker.js's comment is updated to match.Suite (iOS 26.3.1 simulator):
Fixes #503