1144638 - crash in OSXVsyncSource::OSXDisplay::EnableVsync()

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Mark Banner (:standard8)]

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 ?

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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
[..]

Comment 2

[Mason Chang [Inactive] [:mchang]]

(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?

Comment 3

[Mason Chang [Inactive] [:mchang]]

Attachment: 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

Comment 4

[Mark Banner (:standard8)]

It seems to require FF to be started on the external display before unplugging the external display - I've reproduced it 3 times now.

Comment 5

[Markus Stange [:mstange]]

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.

Comment 6

[Mason Chang [Inactive] [:mchang]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/021f1f082494

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/021f1f082494