GeezerVison

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

GeezerVison is showing github.com — fix(worker): tear down a worker runtime whose Java bootstrap failed by adrian-niculescu · Pull Request #2069 · NativeScript/android

Conversation

adrian-niculescu commented Oct 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

A worker whose bootstrap fails after its native runtime is up, for example because internal/ts_helpers.js throws, reports the error to its parent but leaves the native runtime and its isolate allocated for the rest of the process.

Runtime.initWorkerRuntime creates the native runtime in runtime.init() and then runs ts_helpers.js. When that throws, the Java side rolls back its own caches and rethrows, so the call never returns the runtime id, WorkerWrapper::runtime_ stays null, and BackgroundLooper skips the teardown that disposes the isolate. The native runtime is still the thread's current runtime (PrepareV8Runtime sets it last, and UnwindFailedInit clears it when native init itself fails), so shutdown now takes it from there when runtime_ is null and tears it down on the existing path, detaching it from Java by its own id. That id matters when the failure comes after the Java runtime finished initializing, for example a logger that throws on its last line: Java still holds that runtime and its GC subscription, and detaching with the -1 the call never replaced would leave both pointing at the deleted native runtime.

With ts_helpers.js patched to throw in workers, a failed worker on main leaves its runtime current after shutdown, and with this change the runtime is gone and the error still reaches the parent once. That needs a patched asset, so there is no spec for it. The full device suite passes.

Summary by CodeRabbit

Bug Fixes
- Improved cleanup when app startup fails, helping prevent a native runtime from being left undisposed. This includes cases where a native runtime is created but startup does not return its identifier; shutdown can now locate the runtime and complete the existing cleanup steps.

Sorry, something went wrong.

Uh oh!

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

All reactions


          fix(worker): tear down a worker runtime whose Java bootstrap failed

2421b74

When initWorkerRuntime built the native runtime and then failed in Java, for example because internal/ts_helpers.js threw, it threw before returning the runtime id, so the worker never took ownership and the native runtime and its isolate were never disposed. Worker shutdown now takes that runtime from the thread's current one and tears it down like any other.

coderabbitai Bot commented Oct 6, 2026 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

No actionable comments were generated in the recent review.

Walkthrough

WorkerWrapper::BackgroundLooper now recovers the current runtime and its ID during shutdown if Java bootstrap fails before assigning runtime_. The recovered runtime can then follow the existing detach and disposal path.

Changes: Runtime shutdown recovery

Layer / File(s)	Summary
Recover runtime during shutdown `test-app/runtime/src/main/cpp/WorkerWrapper.cpp`	When Java bootstrap fails before assigning `runtime_`, shutdown obtains the current thread’s runtime and ID when available. The existing cleanup path then handles the recovered runtime.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

NativeScript/android#2092: This PR changes the thread-local runtime lookup used by the shutdown recovery path.

Suggested reviewers: edusperoni

Merge Risk: Minimal · up to 11269

This change tears down a worker runtime whose Java bootstrap failed, so the runtime no longer leaks after shutdown. The fallback is narrow and guarded, and no merge-blocking risk was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the change: a fix that tears down a worker runtime when its Java bootstrap fails. The raw summary says BackgroundLooper now recovers the current runtime and its ID during shutdown so t…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the change: a fix that tears down a worker runtime when its Java bootstrap fails. The raw summary says BackgroundLooper now recovers the current runtime and its ID during shutdown so the existing detach and disposal cleanup runs for a partially initialized worker. The objectives describe the same leak and fix. The title is specific and matches the main change.

Finishing Touches

Generate unit tests (beta)

Create a new PR

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

Recent review info

Run configuration

Source mode: Auto

Repository: NativeScript/android

Branch: main

File: .coderabbit.yaml

Commit: 560b3bf

Layers: Repository YAML

Review profile: CHILL

Plan: Advanced

Run ID: `cd71e4e3-1ccd-4c12-94a8-05f97cf5ac74`

Commits

Reviewing files that changed from the base of the PR and between `84d8a99` and `1126907`.

Files selected for processing: 1

`test-app/runtime/src/main/cpp/WorkerWrapper.cpp`

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

All reactions

Sorry, something went wrong.

Uh oh!

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


          fix(worker): detach a recovered worker runtime from Java by its own id

84d8a99

The bootstrap can also fail after the Java runtime finished initializing, when the logger throws on its last line, and the Java side then still holds the runtime and its GC subscription. Detaching with the id the call never returned left both behind, so a later GC notification could reach the deleted native runtime.

adrian-niculescu marked this pull request as ready for review

October 6, 2026 21:53

adrian-niculescu commented Oct 6, 2026

Copy link

Copy Markdown

Contributor Author

@coderabbitai review

All reactions

Sorry, something went wrong.

Uh oh!

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

coderabbitai Bot commented Oct 6, 2026 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

All reactions

Sorry, something went wrong.

Uh oh!

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


          Merge branch 'main' into fix/worker-bootstrap-failure-teardown

This was referenced Nov 11, 2026

fix(worker): keep the worker isolate alive while terminate() uses it, and let it stop a closing worker #2067

Merged

fix(worker): let a heap-limit terminate re-enter the inspector lock from console.log #2070

Merged

edusperoni approved these changes

Nov 11, 2026

View reviewed changes

edusperoni left a comment

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and verified. Reproduced on device with internal/ts_helpers.js patched to throw on worker threads: three failed bootstraps each recovered the runtime, tore it down (current runtime null afterwards), reported exactly one error to the parent and fired nsworkerended, with no crash. Full device suite passes on main + #2067 + #2070 + #2069 (1545 specs, 0 failures, 3 runs).

Sorry, something went wrong.

Uh oh!

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

All reactions

edusperoni merged commit 80f9750 into NativeScript:main

Nov 11, 2026

4 checks passed

Uh oh!

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

edusperoni mentioned this pull request

Nov 11, 2026

fix(worker): drop callbacks from closing workers #2108

Open

Reviewers

edusperoni

edusperoni approved these changes

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.

2 participants

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.

GeezerVison

Conversation

adrian-niculescu commented Oct 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adrian-niculescu commented Oct 6, 2026

Uh oh!

coderabbitai Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edusperoni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adrian-niculescu commented Oct 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

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

coderabbitai Bot commented Oct 6, 2026 •

edited

Loading

Uh oh!

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

coderabbitai Bot commented Oct 6, 2026 •

edited

Loading

Uh oh!

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