Skip to content

devtools: implement pause and resume in debugger#42580

Merged
atbrakhi merged 1 commit intoservo:mainfrom
atbrakhi:pause_and_resume
Feb 18, 2026
Merged

devtools: implement pause and resume in debugger#42580
atbrakhi merged 1 commit intoservo:mainfrom
atbrakhi:pause_and_resume

Conversation

@atbrakhi
Copy link
Copy Markdown
Member

@atbrakhi atbrakhi commented Feb 12, 2026

When a breakpoint is hit, the script thread now pauses execution and
notifies devtools clients with a "paused" event. The script
thread enters a loop that processes devtools messages until
a Resume command is received.

This change does not implement manual pause

Testing: Added new test
Fixes: part of #36027

Screen.Recording.2026-02-17.at.12.22.09.mov

@atbrakhi atbrakhi requested a review from gterzian as a code owner February 12, 2026 13:30
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 12, 2026
Copy link
Copy Markdown
Member

@eerii eerii left a comment

Choose a reason for hiding this comment

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

I think this is going in the right direction, we need a loop like this. But it is blocking all of the messages while being paused, causing panics. For example, if you try to hover over some text while paused, the console actor panics because it is trying to call the evaluate JS message.

We may need to look for another way of blocking execution, but keep the handle_msgs function from script thread running, which is the one handling all of the communication.

Comment thread resources/debugger.js Outdated
Comment thread components/script/script_thread.rs Outdated
@atbrakhi atbrakhi marked this pull request as draft February 12, 2026 15:20
@atbrakhi
Copy link
Copy Markdown
Member Author

atbrakhi commented Feb 12, 2026

I think this is going in the right direction, we need a loop like this. But it is blocking all of the messages while being paused, causing panics. For example, if you try to hover over some text while paused, the console actor panics because it is trying to call the evaluate JS message.

We may need to look for another way of blocking execution, but keep the handle_msgs function from script thread running, which is the one handling all of the communication.

it did not panic for me while performing pause/resume events. I will try out the console events. This needs a bit more thinking i guess!!

@atbrakhi
Copy link
Copy Markdown
Member Author

atbrakhi commented Feb 16, 2026

Previous approach wasn't right. This one feels much better. The enter_debugger_pause_loop might still have room for improvement, suggestions welcome : )

No crashes so far. Will be writing tests to make sure everything holds up!

TODO:

  • Add test for breakpoint pause
  • Update video in description to show pause and resume on breakpoint hit handler

@atbrakhi atbrakhi marked this pull request as ready for review February 17, 2026 11:26
@atbrakhi
Copy link
Copy Markdown
Member Author

@eerii I think this is ready for review now

Copy link
Copy Markdown
Member

@eerii eerii left a comment

Choose a reason for hiding this comment

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

Looks great! :) Ran it locally and both the functionality and the test works.

As we talked, the next step is to unify the handling of breakpoints and manual pausing. For that I think we might need to reorder and rename some functions, but I think it is best to merge this working version now and do the rest incrementally.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Feb 17, 2026
@atbrakhi
Copy link
Copy Markdown
Member Author

Looks great! :) Ran it locally and both the functionality and the test works.

As we talked, the next step is to unify the handling of breakpoints and manual pausing. For that I think we might need to reorder and rename some functions, but I think it is best to merge this working version now and do the rest incrementally.

yess, let's not do the refactoring part in this PR. You are already doing it in #42599, so it would be nice to keep it separate :)

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 18, 2026
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Feb 18, 2026
@atbrakhi atbrakhi enabled auto-merge February 18, 2026 09:44
When a breakpoint is hit, the script thread now pauses execution and
notifies devtools clients with a "paused" event. The script
thread enters a loop that processes devtools messages until
a Resume command is received.

This change does not implement manual pause

Signed-off-by: atbrakhi <atbrakhi@igalia.com>
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 18, 2026
@atbrakhi atbrakhi added this pull request to the merge queue Feb 18, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Feb 18, 2026
Merged via the queue into servo:main with commit 3c8c46d Feb 18, 2026
33 checks passed
@atbrakhi atbrakhi deleted the pause_and_resume branch February 18, 2026 11:26
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Feb 18, 2026
@eerii eerii mentioned this pull request Mar 4, 2026
@delan
Copy link
Copy Markdown
Member

delan commented Mar 23, 2026

excellent work! the screen recording is wonderful too, would it be ok for me to use it in the monthly update? no worries at all if you would prefer not to for any reason :)

@atbrakhi
Copy link
Copy Markdown
Member Author

excellent work! the screen recording is wonderful too, would it be ok for me to use it in the monthly update? no worries at all if you would prefer not to for any reason :)

@delan yeah feel free :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants