crash in OSXVsyncSource::OSXDisplay::EnableVsync()

RESOLVED FIXED in Firefox 39

Status

()

--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: standard8, Assigned: mchang)

Tracking

({crash})

39 Branch
mozilla39
All
Mac OS X
crash
Points:
---

Firefox Tracking Flags

(firefox38 affected, firefox39 fixed)

Details

(crash signature, URL)

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
This bug was filed from the Socorro interface and is 
report bp-641d82f7-1383-41dc-bfd7-e331c2150318.
=============================================================

I just had this crash when I disconnected my external display today.

Using latest nightly (2015-03-17 from https://hg.mozilla.org/mozilla-central/rev/008b3f65a7e0).

Could it be related to bug 1142957 ?
Crash Reason 	EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash Address 	0x0

0 	XUL 	OSXVsyncSource::OSXDisplay::EnableVsync() 	gfx/thebes/gfxPlatformMac.cpp
1 	XUL 	mozilla::gfx::VsyncSource::Display::AddCompositorVsyncDispatcher(mozilla::CompositorVsyncDispatcher*) 	gfx/thebes/VsyncSource.cpp
2 	XUL 	nsRunnableMethodImpl<void (mozilla::CompositorVsyncDispatcher::*)(bool), true, bool>::Run 	obj-firefox/x86_64/dist/include/nsThreadUtils.h
3 	XUL 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
4 	XUL 	NS_ProcessPendingEvents(nsIThread*, unsigned int) 	xpcom/glue/nsThreadUtils.cpp
5 	XUL 	nsBaseAppShell::NativeEventCallback() 	widget/nsBaseAppShell.cpp
6 	XUL 	nsAppShell::ProcessGeckoEvents(void*) 	widget/cocoa/nsAppShell.mm
[..]
status-firefox38: --- → affected
Version: 33 Branch → 39 Branch
(Assignee)

Updated

4 years ago
Assignee: nobody → mchang
Status: NEW → ASSIGNED
(Assignee)

Comment 2

4 years ago
(In reply to Mark Banner (:standard8) from comment #0)
> This bug was filed from the Socorro interface and is 
> report bp-641d82f7-1383-41dc-bfd7-e331c2150318.
> =============================================================
> 
> I just had this crash when I disconnected my external display today.
> 
> Using latest nightly (2015-03-17 from
> https://hg.mozilla.org/mozilla-central/rev/008b3f65a7e0).
> 
> Could it be related to bug 1142957 ?

Yup, it is. Crashing here - https://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxPlatformMac.cpp?from=gfxPlatformMac.cpp&case=true#481

Can you reproduce reliably?
Flags: needinfo?(standard8)
(Assignee)

Comment 3

4 years ago
Created attachment 8579494 [details] [diff] [review]
Retry creating an active display link if a failure occurs

Instead of trying to use the main display or listen to the display configuration notifications, just retry after some amount of time to get the CVDisplayLink.

I tried to listen to the display configuration notifications in OS X, CGDisplayReconfigurationCallBack[1], but you get 2 calls from the callback per display per configuration change. Once with a notification that the display configuration changes, and again when the display configuration is done. However, while testing on a display with 1 external monitor, depending on the port, during the configuration change, we'd get multiple calls that notify a configuration is going to occur and has finished as OS X tries to configure the monitor correctly. Plus, we can still have to deal with the wake from sleep case if we sleep with a monitor attached then wake up without the monitor attached. It would also take a bit of extra logic to make sure that we only try to enable / disable vsync on the main display during a configuration change. I think it's simpler to just try again and get an active display.

[1] https://developer.apple.com/library/mac/documentation/GraphicsImaging/Reference/Quartz_Services_Ref/index.html#//apple_ref/c/tdef/CGDisplayReconfigurationCallBack
Attachment #8579494 - Flags: review?(mstange)
(Reporter)

Comment 4

4 years ago
It seems to require FF to be started on the external display before unplugging the external display - I've reproduced it 3 times now.
Flags: needinfo?(standard8)
Comment on attachment 8579494 [details] [diff] [review]
Retry creating an active display link if a failure occurs

Review of attachment 8579494 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxPlatformMac.cpp
@@ +465,5 @@
> +    static void RetryEnableVsync(nsITimer* aTimer, void* aOsxDisplay)
> +    {
> +      MOZ_ASSERT(NS_IsMainThread());
> +      OSXDisplay* osxDisplay = static_cast<OSXDisplay*>(aOsxDisplay);
> +      MOZ_ASSERT(osxDisplay != nullptr);

" != nullptr" is outlawed.
Attachment #8579494 - Flags: review?(mstange) → review+
https://hg.mozilla.org/mozilla-central/rev/021f1f082494
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.