Skip to content

Conversation

@delan
Copy link
Member

@delan delan commented Jul 23, 2025

in the SpiderMonkey Debugger API, hooks like onNewGlobalObject() use an AutoDebuggerJobQueueInterruption to switch to a new microtask queue and avoid clobbering the debuggee’s microtask queue. this in turn relies on JobQueue::saveJobQueue(), which is not yet implemented in RustJobQueue.

this patch bumps mozjs to servo/mozjs#595, which implements saveJobQueue() and SavedJobQueue for RustJobQueue by calling into Servo via two new JobQueueTraps that create and destroy extra “interrupt” queues for use by the debugger.

SpiderMonkey does not own external job queues, so the lifetime of these queues is managed in Servo, where they are stored in a Vec-based stack. stack-like behaviour is adequate for SpiderMonkey’s save and restore patterns, as far as we can tell, but we’ve added an assertion just in case.

Testing: manually tested working in devtools debugger patch (#37667), where it will undergo automated tests
Fixes: #38311

@delan delan changed the title Implement jsglue traps required for the SpiderMonkey Debugger API Implement jsglue traps for the SpiderMonkey Debugger API Jul 23, 2025
Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
@delan delan force-pushed the mozjs-savejobqueue branch from 4255604 to 1f6c9fa Compare July 23, 2025 10:08
@delan delan changed the title Implement jsglue traps for the SpiderMonkey Debugger API Implement jsglue traps for saveJobQueue() Jul 23, 2025
@atbrakhi atbrakhi changed the title Implement jsglue traps for saveJobQueue() script: Implement jsglue traps for saveJobQueue() Jul 23, 2025
@delan
Copy link
Member Author

delan commented Jul 23, 2025

hmm, clippy is busted in ci, but not locally?

https://github.com/servo/servo/actions/runs/16467745226/job/46549217975

@delan delan marked this pull request as ready for review July 23, 2025 11:29
@delan delan requested a review from gterzian as a code owner July 23, 2025 11:29
@delan delan requested review from jdm and removed request for gterzian July 23, 2025 11:29
Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

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

I'd like to see the code with my suggestions addressed. My hope is that it will raise fewer eyebrows!

@delan delan requested a review from jdm July 25, 2025 07:22
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
Signed-off-by: Delan Azabani <dazabani@igalia.com>
@delan delan force-pushed the mozjs-savejobqueue branch from cacdb8f to c6f599b Compare July 25, 2025 07:44
@delan delan changed the title script: Implement jsglue traps for saveJobQueue() script: Implement jsglue traps for saveJobQueue() Jul 25, 2025
Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

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

Much more clear!

Co-authored-by: atbrakhi <atbrakhi@igalia.com>
Signed-off-by: Delan Azabani <dazabani@igalia.com>
@delan delan force-pushed the mozjs-savejobqueue branch from 167faca to 9f16e31 Compare July 28, 2025 09:48
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
Signed-off-by: Delan Azabani <dazabani@igalia.com>
@delan delan enabled auto-merge July 28, 2025 09:53
Signed-off-by: Delan Azabani <dazabani@igalia.com>
@jdm
Copy link
Member

jdm commented Jul 28, 2025

@delan Clippy warnings are preventing merging.

@delan
Copy link
Member Author

delan commented Jul 28, 2025

@jdm fixed in 77be5d1 :)

@delan delan added this pull request to the merge queue Jul 28, 2025
Merged via the queue into main with commit 9da4c74 Jul 28, 2025
20 of 21 checks passed
@delan delan deleted the mozjs-savejobqueue branch July 28, 2025 12:05
abdelrahman1234567 pushed a commit to abdelrahman1234567/servo that referenced this pull request Jul 28, 2025
in the [SpiderMonkey Debugger
API](https://firefox-source-docs.mozilla.org/js/Debugger/), hooks like
[onNewGlobalObject()](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.html#onnewglobalobject-global)
use an AutoDebuggerJobQueueInterruption to [switch to a new microtask
queue](https://github.com/servo/mozjs/blob/b14aebff23ac4d5b0652060ef949334bda08b22f/mozjs-sys/mozjs/js/src/debugger/Debugger.cpp#L2834-L2841)
and avoid clobbering the debuggee’s microtask queue. this in turn relies
on JobQueue::saveJobQueue(), which is [not yet implemented in
RustJobQueue](https://github.com/servo/mozjs/blob/b14aebff23ac4d5b0652060ef949334bda08b22f/mozjs-sys/src/jsglue.cpp#L83-L86).

this patch bumps mozjs to servo/mozjs#595, which implements
[saveJobQueue() and
SavedJobQueue](https://github.com/servo/mozjs/blob/b14aebff23ac4d5b0652060ef949334bda08b22f/mozjs-sys/mozjs/js/public/Promise.h#L92-L114)
for RustJobQueue by calling into Servo via two new JobQueueTraps that
create and destroy extra “interrupt” queues for use by the debugger.

SpiderMonkey [does not own external job
queues](https://github.com/servo/mozjs/blob/b14aebff23ac4d5b0652060ef949334bda08b22f/mozjs-sys/mozjs/js/public/Promise.h#L117-L123),
so the lifetime of these queues is managed in Servo, where they are
stored in a Vec-based stack. stack-like behaviour is adequate for
SpiderMonkey’s save and restore patterns, as far as we can tell, but
we’ve added an assertion just in case.

Testing: manually tested working in devtools debugger patch (servo#37667),
where it will undergo automated tests
Fixes: servo#38311

---------

Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
minghuaw pushed a commit to minghuaw/servo that referenced this pull request Aug 1, 2025
in the [SpiderMonkey Debugger
API](https://firefox-source-docs.mozilla.org/js/Debugger/), hooks like
[onNewGlobalObject()](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.html#onnewglobalobject-global)
use an AutoDebuggerJobQueueInterruption to [switch to a new microtask
queue](https://github.com/servo/mozjs/blob/b14aebff23ac4d5b0652060ef949334bda08b22f/mozjs-sys/mozjs/js/src/debugger/Debugger.cpp#L2834-L2841)
and avoid clobbering the debuggee’s microtask queue. this in turn relies
on JobQueue::saveJobQueue(), which is [not yet implemented in
RustJobQueue](https://github.com/servo/mozjs/blob/b14aebff23ac4d5b0652060ef949334bda08b22f/mozjs-sys/src/jsglue.cpp#L83-L86).

this patch bumps mozjs to servo/mozjs#595, which implements
[saveJobQueue() and
SavedJobQueue](https://github.com/servo/mozjs/blob/b14aebff23ac4d5b0652060ef949334bda08b22f/mozjs-sys/mozjs/js/public/Promise.h#L92-L114)
for RustJobQueue by calling into Servo via two new JobQueueTraps that
create and destroy extra “interrupt” queues for use by the debugger.

SpiderMonkey [does not own external job
queues](https://github.com/servo/mozjs/blob/b14aebff23ac4d5b0652060ef949334bda08b22f/mozjs-sys/mozjs/js/public/Promise.h#L117-L123),
so the lifetime of these queues is managed in Servo, where they are
stored in a Vec-based stack. stack-like behaviour is adequate for
SpiderMonkey’s save and restore patterns, as far as we can tell, but
we’ve added an assertion just in case.

Testing: manually tested working in devtools debugger patch (servo#37667),
where it will undergo automated tests
Fixes: servo#38311

---------

Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
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.

main is busted with --debug-mozjs / MOZJS_FROM_SOURCE=1

3 participants