1849675 - Debugger Breakpoints Triggered by beforeprint Cause Intermittent Tab Crashes

Status: RESOLVED DUPLICATE of bug 1866385

(DevTools :: Debugger, defect, P2)

Description

[[:Grathium]]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/117.0

Steps to reproduce:

1. Create a function that is called by the window.onbeforeprint event
2. Add a debugger breakpoint on the function called (console.log("testing"); in the example below)
3. Press the "Resume F8" button
4. Observe that the tab crashes ~70% of the time

Example code:

<script>
    function addBreakpointHere() {
        console.log("foo");
    }
    window.addEventListener("beforeprint", addBreakpointHere);
</script>

I have reproduced the issue on both Firefox Developer Edition and Firefox ESR

Actual results:

The active tab crashed

Expected results:

The tab should not have crashed

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'DevTools::Debugger' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Julian Descottes [:jdescottes]]

Thanks! I reproduce the issue.

You can also simply add a debugger statement in the event listener to simplify the STRs.
Here's a crash report for those STRs: https://crash-stats.mozilla.org/report/index/64769dcf-ed20-4ffa-a17f-c14f50230823'

The crash signature is mozilla::CycleCollectedJSContext::SavedMicroTaskQueue::~SavedMicroTaskQueue

Comment 3

[Hubert Boma Manilla (:bomsy)]

(In reply to Julian Descottes [:jdescottes] from comment #2)

Thanks! I reproduce the issue.

You can also simply add a debugger statement in the event listener to simplify the STRs.
Here's a crash report for those STRs: https://crash-stats.mozilla.org/report/index/64769dcf-ed20-4ffa-a17f-c14f50230823'

The crash signature is mozilla::CycleCollectedJSContext::SavedMicroTaskQueue::~SavedMicroTaskQueue

Hi Julien, This STR is windows specific right? I seem to repro slightly differently for Mac.

Adding my steps for Mac with a webpage to repro

STR for Mac

1. Go to https://debugger-crash-before-print.glitch.me/
2. Open debugger
3. Press Command + P to trigger print for the page (This should cause the debugger to pause on line 2)
4. Refresh the webpage
5. Tab Crashes (BUG)

Comment 4

[Julian Descottes [:jdescottes]]

It was reproducing consistently on macos, but I can't seem to reproduce now. Not sure what has possibly changed.

Comment 5

[Tooru Fujisawa [:arai]]

I haven't yet reproduced, but the details so far are the following:

when debugger interrupts the execution, it swaps the debuggee's job queue with temporary one, in order to protect the original job queue while the debuggee is paused.

https://searchfox.org/mozilla-central/rev/08d53deb2cf587e68d1825082c955e8a1926be73/js/public/Promise.h#120

* [SMDOC] Protecting the debuggee's job/microtask queue from debugger activity.

https://searchfox.org/mozilla-central/search?q=AutoDebuggerJobQueueInterruption&path=js%2Fsrc%2Fdebugger%2FDebugger.cpp&case=false&regexp=false

https://searchfox.org/mozilla-central/rev/08d53deb2cf587e68d1825082c955e8a1926be73/js/src/debugger/Debugger.cpp#821-827

bool DebuggerList<HookIsEnabledFun>::dispatchHook(JSContext* cx,
                                                  FireHookFun fireHook) {
  // Preserve the debuggee's microtask event queue while we run the hooks, so
  // the debugger's microtask checkpoints don't run from the debuggee's
  // microtasks, and vice versa.
  JS::AutoDebuggerJobQueueInterruption adjqi;
  if (!adjqi.init(cx)) {

https://searchfox.org/mozilla-central/rev/08d53deb2cf587e68d1825082c955e8a1926be73/js/src/builtin/Promise.cpp#6591,6594

bool JS::AutoDebuggerJobQueueInterruption::init(JSContext* cx) {
...
  saved = cx->jobQueue->saveJobQueue(cx);

https://searchfox.org/mozilla-central/rev/08d53deb2cf587e68d1825082c955e8a1926be73/xpcom/base/CycleCollectedJSContext.cpp#299-300

CycleCollectedJSContext::saveJobQueue(JSContext* cx) {
  auto saved = js::MakeUnique<SavedMicroTaskQueue>(this);

https://searchfox.org/mozilla-central/rev/08d53deb2cf587e68d1825082c955e8a1926be73/xpcom/base/CycleCollectedJSContext.cpp#281-283

explicit SavedMicroTaskQueue(CycleCollectedJSContext* ccjs) : ccjs(ccjs) {
  ccjs->mDebuggerRecursionDepth++;
  ccjs->mPendingMicroTaskRunnables.swap(mQueue);

Newly created jobs are pushed into the temporary job queue, and those jobs are executed by JS::AutoDebuggerJobQueueInterruption::runJobs,

https://searchfox.org/mozilla-central/rev/08d53deb2cf587e68d1825082c955e8a1926be73/js/src/debugger/Debugger.cpp#821,840

bool DebuggerList<HookIsEnabledFun>::dispatchHook(JSContext* cx,
...
      adjqi.runJobs();

https://searchfox.org/mozilla-central/rev/08d53deb2cf587e68d1825082c955e8a1926be73/js/src/builtin/Promise.cpp#6598,6600

void JS::AutoDebuggerJobQueueInterruption::runJobs() {
...
  cx->jobQueue->runJobs(cx);

and when debugger resumes the execution, it restores the job queue.

https://searchfox.org/mozilla-central/rev/08d53deb2cf587e68d1825082c955e8a1926be73/js/public/Promise.h#228,259

class MOZ_RAII JS_PUBLIC_API AutoDebuggerJobQueueInterruption {
...
  js::UniquePtr<JobQueue::SavedJobQueue> saved;

https://searchfox.org/mozilla-central/rev/08d53deb2cf587e68d1825082c955e8a1926be73/xpcom/base/CycleCollectedJSContext.cpp#286-290

~SavedMicroTaskQueue() {
  MOZ_RELEASE_ASSERT(ccjs->mPendingMicroTaskRunnables.empty());
  MOZ_RELEASE_ASSERT(ccjs->mDebuggerRecursionDepth);
  ccjs->mDebuggerRecursionDepth--;
  ccjs->mPendingMicroTaskRunnables.swap(mQueue);

And the assertion failure in comment #2's case happens here because the temporary job queue is not empty, which means at least one promise reaction job is not executed.

So there should be a bug somewhere around the runJobs above, which leaves the pending job in the queue,
and this happens for example when promises reaction jobs are constantly created while the print preview is shown, or some reactions created at the beginning is skipped/overlooked somehow.

Comment 6

[Tooru Fujisawa [:arai]]

I observed the crash by evaluating Promise.resolve(10).then(() => {}) in the console while the debuggee is paused and then resumed it.
it's indeed that the job queue is not empty.

Comment 7

[Julian Descottes [:jdescottes]]

Thanks a lot for the detailed investigation Arai. I can confirm using Bomsy's test page from comment 3 + your console snippet, the tab crash reproduces.

Do you think the fix should rather be on the platform side or in devtools?

Comment 8

[Tooru Fujisawa [:arai]]

it should be fixed in platform, at least not to hit assertion failure.

there might be something needs to be done in devtools around the following call:

https://searchfox.org/mozilla-central/rev/e7b8d13b7513b6fbd97d69e882d7faeed05309d0/devtools/server/actors/utils/event-loop.js#82

xpcInspector.enterNestedEventLoop(this);

Comment 9

[Matthew Gaudet (he/him) [:mgaudet]]

I've got a patch for this up in Bug 1866385, so marking this as a duplicate to avoid double tracking.