FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(runtime): null-initialize Runtime::isolate_ by adrian-niculescu · Pull Request #460 · NativeScript/ios · GitHub

/ ios Public

fix(runtime): null-initialize Runtime::isolate_ - #460

Open
adrian-niculescu wants to merge 1 commit into
NativeScript:mainfrom
adrian-niculescu:fix/runtime-null-isolate
ConversationCommits (1)ChecksFiles changed
Open

fix(runtime): null-initialize Runtime::isolate_#460
adrian-niculescu wants to merge 1 commit into
NativeScript:mainfrom
adrian-niculescu:fix/runtime-null-isolate

Conversation

adrian-niculescu commented Aug 28, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Runtime::isolate_ is only assigned near the end of Init, after the context and its bindings are set up, so between construction and that point it holds an indeterminate value. ~Runtime and GetIsolate() read it unconditionally, and currentRuntime_ already points at the runtime from the constructor on, so anything that reaches the runtime before Init finishes gets garbage rather than null.

This initializes it to nullptr, the same way napiEnv_ next to it already is. No change for a runtime that completed Init.

Summary by CodeRabbit

  • Bug Fixes
    • Improved runtime stability by ensuring internal state is safely initialized before use.

fix(runtime): null-initialize Runtime::isolate_
c3f83a8 
Init assigns isolate_ only after the context and its bindings are set up,
so from construction until then the member holds an indeterminate value.
The destructor and GetIsolate() read it unconditionally, and
currentRuntime_ already points at the runtime from the constructor on.
Initialize it to nullptr, like napiEnv_, so a runtime that has not
completed Init reports no isolate instead of garbage.

coderabbitai Bot commented Aug 28, 2026 •
edited
Loading

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 84f12433-cbba-4169-815b-b42aa0e91084

📥 Commits

Reviewing files that changed from the base of the PR and between 686157b and c3f83a8.

📒 Files selected for processing (1)
  • NativeScript/runtime/Runtime.h

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

📝 Walkthrough

Walkthrough

The Runtime class now initializes isolate_ to nullptr at declaration. This prevents the member from remaining uninitialized before CreateIsolate assigns it.

Changes

Runtime initialization

Layer / File(s) Summary
Initialize isolate pointer
NativeScript/runtime/Runtime.h		 Runtime::isolate_ now defaults to nullptr at declaration.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to c3f83

This localized lifecycle-safety fix gives Runtime::isolate_ a defined null state before initialization without changing behavior after successful setup. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: edusperoni, nathanwalker

Poem

A rabbit checks the runtime's gate

The isolate waits in a safe state
Null before the engines start
A tidy pointer does its part
Hop by hop, the code is clear
No stray values hiding here

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: initializing Runtime::isolate_ to nullptr.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI

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.

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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

Back | FazBrowse Home | New Git URL