GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — fix(runtime): pin the isolate gate at the remaining foreign-thread entries by edusperoni · Pull Request #506 · NativeScript/ios

Conversation

edusperoni commented Oct 9, 2026

Copy link

Copy Markdown

Collaborator

Follows #501. That PR added the per-isolate lifetime gate (IsolateGates, IsolateWrapper::Pin()) and adopted it in ArgConverter::MethodCallback and the JS block dispose. This PR applies the same pattern to the other entry points that can run on a thread other than the isolate's own and that checked IsValid() before waiting on the Locker.

The problem

~Runtime invalidates the isolate without holding the Locker. It then takes the Locker, removes the isolate's Caches, and releases it. Suppose a foreign thread passes IsValid(), then queues on the Locker behind the teardown. When it finally gets the Locker, Caches::Get() returns an empty stand-in, and GetContext() dereferences a null context. If that thread is still waiting when Isolate::Dispose runs, it locks a freed isolate instead.

Converted sites

Every converted site now follows the same steps: pin, bail if the pin is refused or IsValid() is false, take the Locker, re-check IsValid(), then do the work.

Site	Behaviour once the gate is closed or the isolate is invalid
`ArrayAdapter` `count` / `objectAtIndex:`	returns `0` / `nil`
`DictionaryAdapter` `count` / `objectForKey:` / `keyEnumerator`	returns `0` / `nil`
Map and object key enumerators `nextObject` / `allObjects`	returns `nil`
`-dealloc` of `ArrayAdapter`, `DictionaryAdapter`, `NSDataAdapter`	Works like the JS block dispose. While the gate is open, the dealloc detaches its claim, erases `Instances` and resets the persistent, all under the Locker. This happens even after invalidation, because the teardown walk can still reach the claim through the JS object. Once the gate is closed, or the pin is refused, it frees the claim without taking the Locker.
Extended class `+initialize` (ClassBuilder IMP)	skipped
Extended class `retain` / `release` (ClassBuilder IMPs)	Only pins at the retain counts that toggle GC protection. The foreign-thread path no longer reads `Runtime`, which the teardown frees. It posts through `NativeScriptPlatform::LookupEventLoop` and compares against a runloop captured at `__extends` time. The protect/unprotect closures take the Locker before reading `Instances`. If the isolate is gone, the call only forwards to the native retain/release.
Property getter/setter FFI callbacks (`ClassBuilder` extended classes, `MetadataBuilder` JS accessors on native classes)	getter returns a zeroed value, setter does nothing. `PropertyCallbackContext` now carries an `IsolateWrapper`; that is a heap struct, not a block capture.

IsolateWrapper stays trivially copyable, and no ObjC block captures a shared_ptr.

Left alone

These sites only run on the isolate's own thread:

Timers.cpp/.hpp: timers fire from the EventLoop's ordered-lane token drain on the isolate's thread. Their state lives in a Caches state slot that is destroyed under ~Runtime's Locker.
AnimationFrame.mm: the CADisplayLink is added to the creating JS thread's run loop.
Messaging.cpp (TriggerAsync drain, EmitClose, Drain): posted to the port's home loop.
EventLoop::RunEntry, Runtime embedder entry points, DrainRejectionsObserver, AsyncGraphOnFetchCompleted, WorkerWrapper/Worker.mm posts, Helpers.mm Lockers: all run on their own loop or thread.

Repro

IsolateTeardownCallbackTests.js and collectionAdapterQueryWorker.js use a new fixture, +[TNSTestNativeCallbacks query:fromThreads:forMilliseconds:]:

A worker hands a JS array or object to native code.
Native code reads it in a tight loop from 4 background threads.
The worker is terminated 20 ms later, 3 rounds per collection kind.

On main under ASan, the run crashed with a SEGV at address 0 in Caches::GetContext() inside -[ArrayAdapter objectAtIndex:] on a background queue. That crash comes from a read that queued on the Locker behind ~Runtime. The repro is committed on its own, before the fix.

Results

Suite	main (baseline)	this branch
plain	1754 / 0 failures / 11 skipped	1756 / 0 failures / 11 skipped
ASan	1754 / 0 failures / 16 skipped	1756 / 0 failures / 16 skipped

The two extra tests are the new specs.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

edusperoni added 2 commits

October 9, 2026 16:29


          test(runtime): reproduce collection adapters read across worker teardown

da0b7c8

A worker hands a JS array or object to native code that keeps reading
it from four background threads while the worker is terminated. A read
that passed IsolateWrapper::IsValid() and then queued on the isolate's
Locker behind ~Runtime runs after the teardown removed the isolate's
Caches: Caches::Get() returns an empty stand-in, and GetContext()
dereferences its null context (SEGV in -[ArrayAdapter objectAtIndex:]
under ASan). The same wait can also outlast Isolate::Dispose, which
~Runtime does not defer for a thread blocked on the Locker.


          fix(runtime): pin the isolate gate at the remaining foreign-thread en…

efb92ef

…tries

The collection adapters, the extended classes' synthesized methods and
the JS property accessors installed on native classes all checked
IsolateWrapper::IsValid() and then waited for the isolate's Locker. A
thread that passed the check could get the Locker only after ~Runtime
had removed the isolate's Caches, or after Isolate::Dispose, and then
read a null context or a freed isolate.

These entries now follow ArgConverter::MethodCallback: pin the gate,
bail when the pin is refused or the isolate is invalid, take the
Locker, and check validity again under it.

- ArrayAdapter, DictionaryAdapter and its key enumerators: count,
  objectAtIndex:, objectForKey:, keyEnumerator, nextObject and
  allObjects return 0/nil once the isolate is gone.
- Adapter -dealloc (Array, Dictionary, NSData): like the JS block
  dispose, it detaches its claim under the Locker until the gate
  closes, even after the isolate is invalidated, because the teardown
  can still reach the claim through the JS object. Once the gate is
  closed it frees the claim without the Locker.
- Extended class +initialize: skipped once the isolate is gone.
- Extended class retain/release: pin only on the retain counts that
  toggle GC protection. The foreign-thread path posts through
  NativeScriptPlatform::LookupEventLoop and a runloop captured at
  __extends time instead of reading the Runtime, which the teardown
  frees. The protect/unprotect closures take the Locker before they
  read Instances.
- PropertyCallbackContext carries an IsolateWrapper, and the property
  getter/setter callbacks of ClassBuilder and MetadataBuilder pin like
  MethodCallback: a getter returns a zeroed value and a setter does
  nothing once the isolate is gone.

coderabbitai Bot commented Oct 9, 2026

Copy link

Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

edusperoni mentioned this pull request

Oct 13, 2026

fix(runtime): avoid cross-isolate adapter and block teardown deadlocks #514

Open

This branch has not been deployed

No deployments

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

1 participant

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.