1441869 - Windows GfxInfo::Init should (sometimes?) specify DIGCF_DEVICEINTERFACE when calling SetupDiGetClassDevsW

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Geoff Brown [:gbrown]]

In bug 1414495 we have observed on-going intermittent browser startup hangs during a variety of tests. These may have multiple causes.

From https://bugzilla.mozilla.org/show_bug.cgi?id=1414495#c52, a stack dump indicated a hang at

 	[External Code]	
 	xul.dll!mozilla::widget::GfxInfo::Init() Line 393	C++
>	xul.dll!mozilla::widget::GfxInfoConstructor(nsISupports * aOuter, const nsID & aIID, void * * aResult) Line 152	C++
 	xul.dll!nsComponentManagerImpl::CreateInstance(const nsID & aClass, nsISupports * aDelegate, const nsID & aIID, void * * aResult) Line 1003	C++
 	xul.dll!nsComponentManagerImpl::GetService(const nsID & aClass, const nsID & aIID, void * * aResult) Line 1247	C++
 	xul.dll!nsJSCID::GetService(JS::Handle<JS::Value> iidval, JSContext * cx, unsigned char optionalArgc, JS::MutableHandle<JS::Value> retval) Line 702	C++
 	[External Code]	
 	xul.dll!XPC_WN_CallMethod(JSContext * cx, unsigned int argc, JS::Value * vp) Line 929	C++
 	xul.dll!js::InternalCallOrConstruct(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct) Line 473	C++
 	xul.dll!Interpret(JSContext * cx, js::RunState & state) Line 3096	C++
 	xul.dll!js::RunScript(JSContext * cx, js::RunState & state) Line 423	C++
 	xul.dll!js::InternalCallOrConstruct(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct) Line 495	C++
 	xul.dll!Interpret(JSContext * cx, js::RunState & state) Line 3096	C++
 	xul.dll!js::RunScript(JSContext * cx, js::RunState & state) Line 423	C++
 	xul.dll!js::InternalCallOrConstruct(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct) Line 495	C++
 	xul.dll!JS_CallFunctionValue(JSContext * cx, JS::Handle<JSObject *> obj, JS::Handle<JS::Value> fval, const JS::HandleValueArray & args, JS::MutableHandle<JS::Value> rval) Line 2978	C++
 	xul.dll!nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS * wrapper, unsigned short methodIndex, const XPTMethodDescriptor * info_, nsXPTCMiniVariant * nativeParams) Line 1307	C++
 	xul.dll!nsXPCWrappedJS::CallMethod(unsigned short methodIndex, const XPTMethodDescriptor * info, nsXPTCMiniVariant * params) Line 616	C++
 	xul.dll!PrepareAndDispatch(nsXPTCStubBase * self, unsigned int methodIndex, unsigned __int64 * args, unsigned __int64 * gprData, double * fprData) Line 174	C++
 	[External Code]	
 	xul.dll!NS_CreateServicesFromCategory(const char * aCategory, nsISupports * aOrigin, const char * aObserverTopic, const char16_t *) Line 816	C++
 	xul.dll!nsXREDirProvider::DoStartup() Line 1035	C++
 	xul.dll!XREMain::XRE_mainRun() Line 4512	C++
 	xul.dll!XREMain::XRE_main(int argc, char * * argv, const mozilla::BootstrapConfig & aConfig) Line 4808	C++
 	xul.dll!XRE_main(int argc, char * * argv, const mozilla::BootstrapConfig & aConfig) Line 4901	C++


GfxInfo::Init() Line 393 looks like https://searchfox.org/mozilla-central/rev/74b7ffee403c7ffd05b8b476c411cbf11d134eb9/widget/windows/GfxInfo.cpp#393

  HDEVINFO devinfo = SetupDiGetClassDevsW(nullptr, mDeviceID[0].get(), nullptr,
                                          DIGCF_PRESENT | DIGCF_PROFILE | DIGCF_ALLCLASSES);


We also observed:

it looks like mDeviceID[0] is being used as a "PnP device instance ID" and

https://msdn.microsoft.com/en-us/library/windows/hardware/ff551069(v=vs.85).aspx

says

  When specifying a PnP device instance ID, DIGCF_DEVICEINTERFACE must be set in the Flags parameter.

Comment 1

[Geoff Brown [:gbrown]]

I tried setting DIGCF_DEVICEINTERFACE unconditionally but that seemed to cause some new failures:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=699ca312a40c81bbc6d0a939d57696c43df3791e

Comment 2

[Geoff Brown [:gbrown]]

Attachment: set DIGCF_DEVICEINTERFACE when explicitly using a PnP device instance ID

If DIGCF_DEVICEINTERFACE is used only in the case where we explicitly specify a PnP device instance ID, that seems to avoid some test startup hangs and doesn't introduce new failures:

Base (no changes):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b37002ca7fd5762a39e8932c2492269c4160218

With this patch:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8cf2872d9e115368400dde9a9bb3c922b9c4e0d7

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f558cf2238e4
Specify DIGCF_DEVICEINTERFACE when calling SetupDiGetClassDevsW; r=milan

Comment 4

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/f558cf2238e4

Comment 5

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

Geoff, might this change warrant an uplift? Not sure if it is already too late but we could try to request one.

Comment 6

[Geoff Brown [:gbrown]]

It's a good thought, but I won't advocate for uplift at this time because:
 - it's too early to say how effective the fix is (and I suspect there's more to do);
 - it's too early to say the fix hasn't had bad side-effects;
 - I don't know this is definitely a problem affecting users (it could be specific to the test environment somehow).