Skip to content

fix(ext/node): handle multiple calls in inspector.Session.post() - #31067

Merged
bartlomieju merged 2 commits into
denoland:mainfrom
fraidev:fix_node_inspector_nested
Oct 27, 2025
Merged

bartlomieju merged 2 commits into
denoland:mainfrom
fraidev:fix_node_inspector_nested

Conversation

@fraidev

@fraidev fraidev commented Oct 23, 2025 •

Copy link
Copy Markdown
Contributor

Fixes a panic that occurred when using nested session.post() calls in the node:inspector module. The issue manifested when callbacks from inspector commands (like Profiler.enable) called session.post() again, causing a re-entrant invocation of op_inspector_dispatch.

The fix involves:

  1. Marking op_inspector_dispatch as #[op2(fast, reentrant)] to allow re-entrant calls
  2. Implementing a message queue to handle messages that arrive during nested dispatch calls

This ensures that:

  • No panics occur during re-entrant calls
  • All messages are dispatched in the correct order
  • Nested callbacks execute properly, matching Node.js behavior

Includes comprehensive tests for deep nesting, and queue ordering behavior.

Fixes #31020

@fraidev
fraidev force-pushed the fix_node_inspector_nested branch from 7734430 to 0a6de17 Compare October 23, 2025 04:35
@Tango992

Copy link
Copy Markdown
Contributor

Great work! What do you think if we move the queue to the JS side to make it less complex on the Rust side? I was thinking something like this: b258bb4

@bartlomieju

Copy link
Copy Markdown
Member

I agree with @Tango992 here - seems to be much simpler to keep it in JS

@fraidev
fraidev force-pushed the fix_node_inspector_nested branch 2 times, most recently from cb3b182 to 28c9eaf Compare October 25, 2025 14:27
@fraidev
fraidev force-pushed the fix_node_inspector_nested branch from 28c9eaf to b732423 Compare October 25, 2025 14:44
@fraidev

fraidev commented Oct 25, 2025

Copy link
Copy Markdown
Contributor Author

Great work! What do you think if we move the queue to the JS side to make it less complex on the Rust side? I was thinking something like this: b258bb4

Makes sense! I moved the queue to the JS side, and all tests are still passing.

@bartlomieju bartlomieju left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great, thanks!

@bartlomieju
bartlomieju merged commit c0ebacb into denoland:main Oct 27, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Deno has panicked" when using inspector.Session()

3 participants