1412635 - Crash in MessageBuilder::WriteElement

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect, P1)

Description

[[:philipp]]

[Tracking Requested - why for this release]:
these signatures jumped up in volume in 57.0b12 where they account for 20% of browser crashes.

This bug was filed from the Socorro interface and is 
report bp-ed8ca1ba-af47-4fd5-b1f5-34dd50171028.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	uiautomationcore.dll 	MessageBuilder::WriteElement(IUiaNode*, IServerConnection*) 	
1 	uiautomationcore.dll 	MessageBuilder::WriteTraverseStateOut(TraverseStateOut*, IServerConnection*) 	
2 	uiautomationcore.dll 	RemoteUiaNodeStub::Incoming_Find(UiaNode*, ITargetContextInvoker*, IServerConnection*, MessageParser&, MessageBuilder&) 	
3 	uiautomationcore.dll 	RemoteUiaNodeStub::OnMessage(IUnknown*, ITargetContextInvoker*, IServerConnection*, Protocol_MethodId, MessageParser&, MessageBuilder&) 	
4 	uiautomationcore.dll 	InvokeOnCorrectContext_Callback(void*) 	
5 	uiautomationcore.dll 	ProcessIncomingRequest(MessageParser&, MessageBuilder&, IServerConnection*) 	
6 	uiautomationcore.dll 	HookBasedServerConnectionManager::HookCallback(void*, unsigned long, void**, unsigned long*, void**) 	
7 	uiautomationcore.dll 	HookUtil<&HookBasedClientConnection::HookCallback(void*, unsigned long, void**, unsigned long*, void**), 0>::CallOut(void*, unsigned long, void**, unsigned long*, void**) 	
8 	uiautomationcore.dll 	HandleHookMessage(tagCWPSTRUCT*, unsigned long, void (*)(void*, unsigned long, void**, unsigned long*, void**), void (*)(int, void*)) 	
9 	uiautomationcore.dll 	HookUtil<&HookBasedClientConnection::HookCallback(void*, unsigned long, void**, unsigned long*, void**), 0>::CallWndProc(int, unsigned __int64, __int64) 	
10 	user32.dll 	DispatchHookW 	
11 	user32.dll 	fnHkINLPCWPSTRUCTW 	
12 	user32.dll 	_fnDWORD 	
13 	ntdll.dll 	KiUserCallbackDispatch 	
14 	user32.dll 	ZwUserPeekMessage 	
15 	user32.dll 	PeekMessageW 	
16 	msctf.dll 	CThreadInputMgr::PeekMessageW(tagMSG*, HWND__*, unsigned int, unsigned int, unsigned int, int*) 	
17 	xul.dll 	nsAppShell::ProcessNextNativeEvent(bool) 	widget/windows/nsAppShell.cpp:319
18 	xul.dll 	nsBaseAppShell::DoProcessNextNativeEvent(bool) 	widget/nsBaseAppShell.cpp:140
19 	xul.dll 	nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, bool) 	widget/nsBaseAppShell.cpp:291
20 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:952
21 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/threads/nsThreadUtils.cpp:521
22 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:125
23 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:319
24 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:299
25 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp:158
26 	xul.dll 	nsAppShell::Run() 	widget/windows/nsAppShell.cpp:230
27 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/nsAppStartup.cpp:288
28 	xul.dll 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp:4703
29 	xul.dll 	XREMain::XRE_main(int, char** const, mozilla::BootstrapConfig const&) 	toolkit/xre/nsAppRunner.cpp:4867
30 	xul.dll 	XRE_main(int, char** const, mozilla::BootstrapConfig const&) 	toolkit/xre/nsAppRunner.cpp:4962
31 	firefox.exe 	NS_internal_main(int, char**, char**) 	browser/app/nsBrowserApp.cpp:309
32 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:115
33 	firefox.exe 	__scrt_common_main_seh 	f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:253
34 	kernel32.dll 	BaseThreadInitThunk 	
35 	ntdll.dll 	RtlUserThreadStart

these accessibility crashes are already around in a lower volume on 52esr but they spiked up for windows 7 and windows 8 installations in nightly 58.0a1 build 20171024100135 & 57.0b12. a common patch landing in those two versions would be bug 1383131.

user comments indicate that the browser is constantly hanging/freezing for them and according to correlations this may be somewhat tied to the third-party baidu antivirus program:

Signature report for MessageBuilder::WriteTraverseStateOut
Correlations for Firefox Beta
(99.20% in signature vs 02.79% overall) Module "uiautomationcore.dll" = true
(98.40% in signature vs 03.71% overall) reason = EXCEPTION_ACCESS_VIOLATION_EXEC
(87.20% in signature vs 00.41% overall) Module "BavUm.dll" = true [82.09% vs 00.38% if platform_version = 6.1.7601 Service Pack 1]
(87.20% in signature vs 00.41% overall) Module "Bavnt.dll" = true [82.09% vs 00.38% if platform_version = 6.1.7601 Service Pack 1]
(58.40% in signature vs 00.13% overall) Module "BavCommon.dll" = true
(45.60% in signature vs 01.45% overall) Module "webio.dll" = true
(16.00% in signature vs 00.95% overall) cpu_microcode_version = 0x3 [22.99% vs 02.05% if cpu_arch = null]
(19.20% in signature vs 01.75% overall) platform_pretty_version = Windows 8

Signature report for MessageBuilder::WriteElement
Correlations for Firefox Beta
(100.0% in signature vs 09.52% overall) Module "ia2marshal.dll" = true
(98.88% in signature vs 08.31% overall) Module "AccessibleHandler.dll" = true
(25.84% in signature vs 00.41% overall) Module "BavUm.dll" = true [81.48% vs 05.31% if platform_pretty_version = Windows 8]
(25.84% in signature vs 00.41% overall) Module "Bavnt.dll" = true [81.48% vs 05.35% if platform_version = 6.2.9200]
(40.45% in signature vs 00.09% overall) Module "dtvhooks64.dll" = true [56.45% vs 00.16% if platform_version = 6.1.7601 Service Pack 1]
(17.98% in signature vs 00.13% overall) Module "BavCommon.dll" = true [55.56% vs 02.59% if platform_version = 6.2.9200]
(30.34% in signature vs 01.75% overall) platform_pretty_version = Windows 8
(100.0% in signature vs 10.97% overall) accessibility = Active [166.67% vs 11.10% if process_type = null]
(16.85% in signature vs 00.08% overall) Module "BavUm64.dll" = true [22.58% vs 00.09% if platform_version = 6.1.7601 Service Pack 1]
(16.85% in signature vs 00.08% overall) Module "Bavnt64.dll" = true [22.58% vs 00.09% if platform_version = 6.1.7601 Service Pack 1]

Comment 1

[Marcia Knous [:marcia]]

Tracking 57+ based on the volume increase in B12.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Disable hack for UIA

This patch disables the compat hack for "pure" UIA (ie, no other client bits set).

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 8923500 [details] [diff] [review]
Disable hack for UIA

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

r=me thanks. Should we follow up with MS on the crashes?

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to David Bolter [:davidb] (NeedInfo me for attention) from comment #3) 
> r=me thanks. Should we follow up with MS on the crashes?

Given that this compat hack intentionally breaks some rules, probably not ;-)

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e29ef925886304fb2008b6cedce84d76499aa93d
Bug 1412635: Disable InSendMessageEx compat hack for UIA; r=davidb

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance. I'm less sold on ESR52 given the volume, but feel free to do so if you feel it's worth doing.

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8923500 [details] [diff] [review]
Disable hack for UIA

Approval Request Comment
[Feature/Bug causing the regression]: bug 1383131
[User impact if declined]: Crashing on some Windows 7 and Windows 8 machines.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: No, because we don't have clear STR. This patch is speculative but is also so trivial that the risk is extremely low.
[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?]: No
[Why is the change risky/not risky?]: Trivial patch, just changes a flag.
[String changes made/needed]: No

Comment 9

[Eliza Balazs [:ebalazs_]]

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

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8923500 [details] [diff] [review]
Disable hack for UIA

Top crasher, Beta57+

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3060bafa83a9