fix: enable interruption of V8 execution on Debugger.pause command#378
fix: enable interruption of V8 execution on Debugger.pause command#378edusperoni wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR adds interrupt-based pause handling to the NativeScript V8 inspector client. When a ChangesV8 Debugger pause interrupt handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@NativeScript/inspector/JsV8InspectorClient.mm`:
- Around line 131-141: The RequestInterrupt path for "Debugger.pause" should be
skipped when the inspector is already in a paused nested loop; wrap the existing
parsed-check and isolate_->RequestInterrupt call in a guard that only runs when
runningNestedLoops_ is false (i.e., if (!runningNestedLoops_) { ... }), so that
runMessageLoopOnPause() (which sets runningNestedLoops_ = true) continues to
handle pause messages via session_->schedulePauseOnNextStatement rather than
queuing an interrupt that re-pauses immediately; update the branch in
JsV8InspectorClient around the RequestInterrupt lambda to check
runningNestedLoops_ before calling isolate_->RequestInterrupt.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c306d087-23a0-4ae6-b87a-92dfe7f5b5b1
📒 Files selected for processing (1)
NativeScript/inspector/JsV8InspectorClient.mm
| // Debugger.pause needs to interrupt V8 even if the main thread is busy | ||
| // executing JS. RequestInterrupt fires at the next safe bytecode boundary. | ||
| auto parsed = json::parse(message, nullptr, false); | ||
| if (!parsed.is_discarded() && parsed.contains("method") && parsed["method"] == "Debugger.pause") { | ||
| isolate_->RequestInterrupt( | ||
| [](Isolate* isolate, void* data) { | ||
| auto client = static_cast<JsV8InspectorClient*>(data); | ||
| client->session_->schedulePauseOnNextStatement({}, {}); | ||
| }, | ||
| this); | ||
| } |
There was a problem hiding this comment.
Skip the interrupt path when the inspector is already paused.
runMessageLoopOnPause() already pumps frontend messages with runningNestedLoops_ = true. In that state no JS is executing, so this RequestInterrupt stays pending until after resume and can cause an unexpected immediate re-pause on the next statement. Gate the new branch on !runningNestedLoops_ and let the paused message loop handle Debugger.pause normally.
Suggested guard
auto parsed = json::parse(message, nullptr, false);
- if (!parsed.is_discarded() && parsed.contains("method") && parsed["method"] == "Debugger.pause") {
+ bool shouldInterrupt = false;
+ dispatch_sync(this->messageLoopQueue_, ^{
+ shouldInterrupt = !runningNestedLoops_;
+ });
+ if (shouldInterrupt && !parsed.is_discarded() && parsed.contains("method") &&
+ parsed["method"] == "Debugger.pause") {
isolate_->RequestInterrupt(
[](Isolate* isolate, void* data) {
auto client = static_cast<JsV8InspectorClient*>(data);
client->session_->schedulePauseOnNextStatement({}, {});
},
this);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@NativeScript/inspector/JsV8InspectorClient.mm` around lines 131 - 141, The
RequestInterrupt path for "Debugger.pause" should be skipped when the inspector
is already in a paused nested loop; wrap the existing parsed-check and
isolate_->RequestInterrupt call in a guard that only runs when
runningNestedLoops_ is false (i.e., if (!runningNestedLoops_) { ... }), so that
runMessageLoopOnPause() (which sets runningNestedLoops_ = true) continues to
handle pause messages via session_->schedulePauseOnNextStatement rather than
queuing an interrupt that re-pauses immediately; update the branch in
JsV8InspectorClient around the RequestInterrupt lambda to check
runningNestedLoops_ before calling isolate_->RequestInterrupt.
2 participants
Summary by CodeRabbit