1895175 - Crash in [@ _dispatch_client_callout3]

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P3)

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/3e7f0bf5-ec50-4e57-a449-9d4a30240402

MOZ_CRASH Reason: MOZ_CRASH(Unhandled exception)

Top 10 frames:

0  XUL  CrashReporter::TerminateHandler()  toolkit/crashreporter/nsExceptionHandler.cpp:1866
1  libc++abi.dylib  std::__terminate(void (*)())
2  libc++abi.dylib  std::terminate()
3  libobjc.A.dylib  objc_terminate
4  libdispatch.dylib  _dispatch_client_callout3
5  libdispatch.dylib  _dispatch_mach_msg_async_reply_invoke
6  libdispatch.dylib  _dispatch_main_queue_drain
7  libdispatch.dylib  _dispatch_main_queue_callback_4CF
8  CoreFoundation  __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__
9  CoreFoundation  __CFRunLoopRun

Low-volume crash, there's at least two comments mentioning playing video and one has something that looks like a potential STR:

I’m on macOS with the title bar hidden by default. When watching videos, the title bar stops appearing when I mouse up to the top of the screen. After that, entering full screen video will crash Firefox.

Comment 1

[Brad Werth [:bradwerth]]

Not able to reproduce with the steps in comment 0, but I'll keep trying some things.

Comment 2

[Brad Werth [:bradwerth]]

I think this is not specific to video because many of the crash reports have URLs that are not video. I do think it's related to fullscreen, though, also mention in comment 0. I think the callstack shows a runnable that is trying to dispatch to a null pointer. That's a guess, but I think that's what it's telling us. There is only one dispatch in nsCocoaWindow (other than emulated fullscreen, which is now preffed-off by default) and it is in FinishCurrentTransitionIfMatching which will be dispatched at the end of asynchronous transitions like fullscreen or minimization.

After the dispatch, it's entirely possible that the nsCocoaWindow is deallocated before the dispatched callback runs. I'm not sure what to do about this. One option is to dispatch instead to an object that isn't deallocated, that tracks the nsCocoaWindow objects and will invoke the appropriate callback on an object if it's still alive. That's kind of gross, but it's the best idea I have. I'll try to put a patch together.

Comment 3

[Brad Werth [:bradwerth]]

(In reply to Brad Werth [:bradwerth] from comment #2)

One option is to dispatch instead to an object that isn't deallocated, that tracks the nsCocoaWindow objects and will invoke the appropriate callback on an object if it's still alive. That's kind of gross, but it's the best idea I have. I'll try to put a patch together.

Better idea. We have NS_NewCancelableRunnableFunction for this purpose. Much easier to integrate into the existing logic.

Comment 4

[Brad Werth [:bradwerth]]

Attachment: Bug 1895175: Make dispatched calls to nsCocoaWindow::ProcessTransitions cancellable.

This prevents one more way that ProcessTransitions might be called
after we'd rather it didn't. The CancelAllTransitions method is
updated to also cancel a dispatched call to ProcessTransitions. This
will prevent these dispatches from arriving after the nsCocoaWindow
has been deallocated.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8a1f0e8bdf88
Make dispatched calls to nsCocoaWindow::ProcessTransitions cancellable. r=mac-reviewers,spohl

Comment 6

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/8a1f0e8bdf88

Comment 7

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

The patch landed in nightly and beta is affected.
:bradwerth, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox128 to wontfix.

For more information, please visit BugBot documentation.

Comment 8

[Brad Werth [:bradwerth]]

This is low-volume and it should ride the trains.