Closed Bug 1390002 Opened 7 years ago Closed 7 years ago

startup Crash in mozilla::gfx::VsyncSource::GetRefreshTimerVsyncDispatcher

Categories

(Core :: Graphics, defect)

x86
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- fixed
firefox55 --- wontfix
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: wsmwk, Assigned: mchang)

References

Details

(Keywords: crash, Whiteboard: [startupcrash])

Crash Data

Attachments

(2 files)

99% of crashes are all esr crashes, covering all versions of 52esr - https://crash-stats.mozilla.com/search/?signature=~mozilla%3A%3Agfx%3A%3AVsyncSource%3A%3AGetRefreshTimerVsyncDispatcher&product=Firefox&date=%3E%3D2017-02-14T00%3A02%3A39.000Z&date=%3C2017-08-14T00%3A02%3A39.000Z&_sort=uptime&_sort=build_id&_sort=-date&_facets=signature&_facets=version&_facets=release_channel&_facets=total_virtual_memory&_facets=platform_version&_columns=date&_columns=version&_columns=build_id&_columns=platform&_columns=uptime#facet-version

This bug was filed from the Socorro interface and is 
report bp-e806443b-66bd-45b3-b222-a8e710170809.
=============================================================
 0 	xul.dll	mozilla::gfx::VsyncSource::GetRefreshTimerVsyncDispatcher()	gfx/thebes/VsyncSource.cpp:39
1 	xul.dll	mozilla::VsyncRefreshDriverTimer::VsyncRefreshDriverTimer()	layout/base/nsRefreshDriver.cpp:427
2 	xul.dll	CreateVsyncRefreshTimer	layout/base/nsRefreshDriver.cpp:943
3 	xul.dll	nsRefreshDriver::ChooseTimer()	layout/base/nsRefreshDriver.cpp:1077
4 	xul.dll	nsRefreshDriver::EnsureTimerStarted(nsRefreshDriver::EnsureTimerStartedFlags)	layout/base/nsRefreshDriver.cpp:1298
5 	xul.dll	nsRefreshDriver::AddRefreshObserver(nsARefreshObserver*, mozFlushType)	layout/base/nsRefreshDriver.cpp:1206
6 	xul.dll	nsIPresShell::AddRefreshObserverInternal(nsARefreshObserver*, mozFlushType)	layout/base/nsPresShell.cpp:9810
7 	xul.dll	mozilla::a11y::NotificationController::ScheduleProcessing()	accessible/base/NotificationController.cpp:447
8 	xul.dll	mozilla::a11y::NotificationController::NotificationController(mozilla::a11y::DocAccessible*, nsIPresShell*)	accessible/base/NotificationController.cpp:34
9 	xul.dll	mozilla::a11y::DocAccessible::Init()	accessible/generic/DocAccessible.cpp:415
10 	xul.dll	mozilla::a11y::DocManager::CreateDocOrRootAccessible(nsIDocument*)	accessible/base/DocManager.cpp:487
11 	xul.dll	mozilla::a11y::DocManager::GetDocAccessible(nsIDocument*)	accessible/base/DocManager.cpp:67
12 	xul.dll	mozilla::a11y::ApplicationAccessible::Init()	accessible/generic/ApplicationAccessible.cpp:186
13 	xul.dll	nsAccessibilityService::Init()	accessible/base/nsAccessibilityService.cpp:1284
14 	xul.dll	GetOrCreateAccService(unsigned int)	accessible/base/nsAccessibilityService.cpp:1792
15 	xul.dll	nsBaseWidget::GetRootAccessible()	widget/nsBaseWidget.cpp:1919
16 	xul.dll	nsWindow::GetAccessible()	widget/windows/nsWindow.cpp:7270
17 	xul.dll	nsWindow::ProcessMessage(unsigned int, unsigned int&, long&, long*)	widget/windows/nsWindow.cpp:5805
18 	xul.dll	nsWindow::WindowProcInternal(HWND__*, unsigned int, unsigned int, long)	widget/windows/nsWindow.cpp:4829
19 	xul.dll	CallWindowProcCrashProtected	xpcom/base/nsCrashOnException.cpp:35
20 	xul.dll	nsWindow::WindowProc(HWND__*, unsigned int, unsigned int, long)	widget/windows/nsWindow.cpp:4781
21 	user32.dll	_InternalCallWinProc	
22 	user32.dll	UserCallWinProcCheckWow	
23 	user32.dll	DispatchClientMessage	
24 	user32.dll	__fnDWORD	
25 	ntdll.dll	KiUserCallbackDispatcher	
26 	ntdll.dll	KiUserApcDispatcher	
27 	xul.dll	nsScreenWin::GetRect(int*, int*, int*, int*)	widget/windows/nsScreenWin.cpp:62
28 	xul.dll	gfxPlatform::PopulateScreenInfo()	gfx/thebes/gfxPlatform.cpp:1239
29 	xul.dll	gfxPlatform::Init()	gfx/thebes/gfxPlatform.cpp:710
30 	xul.dll	gfxPlatform::GetPlatform()	gfx/thebes/gfxPlatform.cpp:534
31 	xul.dll	mozilla::widget::GfxInfo::GetDWriteEnabled(bool*)	widget/windows/GfxInfo.cpp:69
32 	xul.dll	NS_InvokeByIndex	xpcom/reflect/xptcall/md/win32/xptcinvoke_asm_x86_msvc.asm:54
33 	xul.dll	XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode)	js/xpconnect/src/XPCWrappedNative.cpp:1344
34 	xul.dll	XPC_WN_GetterSetter(JSContext*, unsigned int, JS::Value*)	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1036
35 	xul.dll	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)	js/src/vm/Interpreter.cpp:459
36 	xul.dll	CallGetter	js/src/vm/NativeObject.cpp:1806
37 	xul.dll	js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)	js/src/jsobj.h:854
38 	xul.dll	Interpret	js/src/vm/Interpreter.cpp:2760
39 	xul.dll	js::RunScript(JSContext*, js::RunState&)	js/src/vm/Interpreter.cpp:405
40 	xul.dll	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)	js/src/vm/Interpreter.cpp:477
41 	xul.dll	CallGetter	js/src/vm/NativeObject.cpp:1806
42 	xul.dll	js::Wrapper::get(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)	js/src/proxy/Wrapper.cpp:143
43 	xul.dll	js::CrossCompartmentWrapper::get(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)	js/src/proxy/CrossCompartmentWrapper.cpp:209
44 	xul.dll	js::Proxy::get(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)	js/src/proxy/Proxy.cpp:309
45 	xul.dll	js::proxy_GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)	js/src/proxy/Proxy.cpp:582
46 	xul.dll	GetPropertyOperation	js/src/vm/Interpreter.cpp:192
47 	xul.dll	Interpret	js/src/vm/Interpreter.cpp:2639
48 	xul.dll	js::RunScript(JSContext*, js::RunState&)	js/src/vm/Interpreter.cpp:405
49 	xul.dll	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)	js/src/vm/Interpreter.cpp:477
50 	xul.dll	InternalCall	js/src/vm/Interpreter.cpp:504
51 	xul.dll	nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*)	js/xpconnect/src/XPCWrappedJSClass.cpp:1214
52 	xul.dll	nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*)	js/xpconnect/src/XPCWrappedJS.cpp:613
53 	xul.dll	PrepareAndDispatch	xpcom/reflect/xptcall/md/win32/xptcstubs.cpp:85
54 	xul.dll	SharedStub	xpcom/reflect/xptcall/md/win32/xptcstubs.cpp:112
55 	xul.dll	NS_CreateServicesFromCategory(char const*, nsISupports*, char const*, char16_t const*)	xpcom/components/nsCategoryManager.cpp:824
Startup+ESR, it'd be good to know if we can work around this.
Flags: needinfo?(mchang)
Flags: needinfo?(mchang)
See Also: → 1388546
Assignee: nobody → mchang
Looks like there are some paths to find out if dwrite is enabled, which inits gfxPlatform, which requires information from PopulateScreenInfo[1], which depends on the refresh driver existing. At this time however, we haven't initialized our vsync source so we crash. This moves vsync initialization earlier in the gfx pipeline.

[1] http://searchfox.org/mozilla-central/source/gfx/thebes/gfxPlatform.cpp#749
Attachment #8897469 - Flags: review?(bugmail)
Attachment #8897469 - Attachment description: Init RefreshDriver before PopulateScreenInfo → Init VsyncSource before PopulateScreenInfo
Comment on attachment 8897469 [details] [diff] [review]
Init VsyncSource before PopulateScreenInfo

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

Put comment 2 into the commit message.
Attachment #8897469 - Flags: review?(bugmail) → review+
Pushed by mchang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2241628188e4
There are some paths to find out if dwrite is enabled, which inits gfxPlatform, which requires information from PopulateScreenInfo, which depends on the refresh driver existing. At this time however, we haven't initialized our vsync source so we crash. This moves vsync initialization earlier in the gfx pipeline. r=kats
https://hg.mozilla.org/mozilla-central/rev/2241628188e4
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment on attachment 8897469 [details] [diff] [review]
Init VsyncSource before PopulateScreenInfo

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined:  Startup Crash
Fix Landed on Version: Firefox 57
Risk to taking this patch (and alternatives if risky): See blow
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Approval Request Comment
[Feature/Bug causing the regression]: Unsure, vsync has been around since Firefox 40 or so.
[User impact if declined]: User startup crash
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: No, as this crash only happened in ESR.
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Moderate
[Why is the change risky/not risky?]: Generally, vsync is always required to run Firefox. However, the ordering of things in startup isn't as defined as other parts of the system and this could cause random changes in timing that wasn't apparent before. This just moves initialization of one component from early in startup to slightly earlier in startup.
[String changes made/needed]: None
Attachment #8897469 - Flags: approval-mozilla-esr52?
Attachment #8897469 - Flags: approval-mozilla-beta?
Attachment #8897469 - Flags: approval-mozilla-esr52?
Hi :mchang,
If the crash only happened in ESR, why do we need to uplift this to Beta56? Can we let this ride the train and not fix in 56?
Flags: needinfo?(mchang)
(In reply to Gerry Chang [:gchang] from comment #9)
> Hi :mchang,
> If the crash only happened in ESR, why do we need to uplift this to Beta56?
> Can we let this ride the train and not fix in 56?

Because of bug 1388546.

This is a topcrash in Thunderbird 56 - #1 - so we'd appreciate if it can be uplifted to beta ASAP
https://crash-stats.mozilla.com/topcrashers/?product=Thunderbird&version=56.0b2
Blocks: 1388546
See Also: 1388546
(In reply to Gerry Chang [:gchang] from comment #9)
> Hi :mchang,
> If the crash only happened in ESR, why do we need to uplift this to Beta56?
> Can we let this ride the train and not fix in 56?

And it's still a start up crash. I still saw some crashes in 56 when digging through the crash reports so it'd be nice to fix those.
Flags: needinfo?(mchang)
Comment on attachment 8897469 [details] [diff] [review]
Init VsyncSource before PopulateScreenInfo

Fix a startup crash. Beta56+.
Attachment #8897469 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8897893 [details] [diff] [review]
Init VsyncSource Before PopulatScreenInfo - Rebased ESR 52

fix a startup crash on esr52.4
Attachment #8897893 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
You need to log in before you can comment on or make changes to this bug.