992751 - Crash when mozilla::widget::NativeKey::GetFollowingCharMessage(tagMSG&) calls PeekMessage() and that causes nested key message handling

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect, P5)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Finally, we find an evidence of the cause of strange message queue behavior.

https://crash-stats.mozilla.com/report/index/6a2dbd0b-fc4e-4a32-8ddc-b0a852140325

> xul.dll	mozilla::widget::NativeKey::GetFollowingCharMessage(tagMSG &)
> xul.dll	mozilla::widget::NativeKey::HandleKeyDownMessage(bool *)
> xul.dll	nsWindow::ProcessKeyDownMessage(tagMSG const &,bool *)
> xul.dll	nsWindow::ProcessMessage(unsigned int,unsigned int &,long &,long *)
> xul.dll	nsWindow::WindowProcInternal(HWND__ *,unsigned int,unsigned int,long)
> xul.dll	CallWindowProcCrashProtected
> xul.dll	nsWindow::WindowProc(HWND__ *,unsigned int,unsigned int,long)
> user32.dll	InternalCallWinProc
> user32.dll	UserCallWinProcCheckWow
> user32.dll	SendMessageWorker
> user32.dll	SendMessageW
> pgdll.dll	pgdll.dll@0x30f8
> 	@0x103
> user32.dll	NtUserPeekMessage
> user32.dll	_PeekMessage
> xul.dll	mozilla::widget::WinUtils::PeekMessageW(tagMSG *,HWND__ *,unsigned int,unsigned int,unsigned int)
> xul.dll	mozilla::widget::NativeKey::IsFollowedByDeadCharMessage()
> xul.dll	mozilla::widget::NativeKey::NativeKey(nsWindowBase *,tagMSG const &,mozilla::widget::ModifierKeyState const &,nsTArray<mozilla::widget::NativeKey::FakeCharMsg> *)
> xul.dll	nsWindow::ProcessKeyDownMessage(tagMSG const &,bool *)
> xul.dll	nsWindow::ProcessMessage(unsigned int,unsigned int &,long &,long *)
> xul.dll	nsWindow::WindowProcInternal(HWND__ *,unsigned int,unsigned int,long)
> xul.dll	CallWindowProcCrashProtected
> xul.dll	nsWindow::WindowProc(HWND__ *,unsigned int,unsigned int,long)
> user32.dll	InternalCallWinProc
> user32.dll	UserCallWinProcCheckWow
> user32.dll	SendMessageWorker
> user32.dll	SendMessageW
> pgdll.dll	pgdll.dll@0x30f8
> 	@0x103
> user32.dll	NtUserPeekMessage
> user32.dll	_PeekMessage
> xul.dll	mozilla::widget::WinUtils::PeekMessageW(tagMSG *,HWND__ *,unsigned int,unsigned int,unsigned int)
> xul.dll	mozilla::widget::NativeKey::IsFollowedByDeadCharMessage()
> xul.dll	mozilla::widget::NativeKey::NativeKey(nsWindowBase *,tagMSG const &,mozilla::widget::ModifierKeyState const &,nsTArray<mozilla::widget::NativeKey::FakeCharMsg> *)
> xul.dll	nsWindow::ProcessKeyDownMessage(tagMSG const &,bool *)
> xul.dll	nsWindow::ProcessMessage(unsigned int,unsigned int &,long &,long *)
> xul.dll	nsWindow::WindowProcInternal(HWND__ *,unsigned int,unsigned int,long)
> xul.dll	CallWindowProcCrashProtected
> xul.dll	nsWindow::WindowProc(HWND__ *,unsigned int,unsigned int,long)
> user32.dll	InternalCallWinProc
> user32.dll	UserCallWinProcCheckWow
> user32.dll	SendMessageWorker
> user32.dll	SendMessageW
> pgdll.dll	pgdll.dll@0x30f8
> 	@0x103
> user32.dll	NtUserPeekMessage
> user32.dll	_PeekMessage
> xul.dll	mozilla::widget::WinUtils::PeekMessageW(tagMSG *,HWND__ *,unsigned int,unsigned int,unsigned int)
> xul.dll	mozilla::widget::NativeKey::IsFollowedByDeadCharMessage()
> xul.dll	mozilla::widget::NativeKey::NativeKey(nsWindowBase *,tagMSG const &,mozilla::widget::ModifierKeyState const &,nsTArray<mozilla::widget::NativeKey::FakeCharMsg> *)
> xul.dll	nsWindow::ProcessKeyDownMessage(tagMSG const &,bool *)
> xul.dll	nsWindow::ProcessMessage(unsigned int,unsigned int &,long &,long *)
> xul.dll	nsWindow::WindowProcInternal(HWND__ *,unsigned int,unsigned int,long)
> xul.dll	CallWindowProcCrashProtected
> xul.dll	nsWindow::WindowProc(HWND__ *,unsigned int,unsigned int,long)
> user32.dll	InternalCallWinProc
> user32.dll	UserCallWinProcCheckWow
> user32.dll	SendMessageWorker
> user32.dll	SendMessageW
> pgdll.dll	pgdll.dll@0x30f8
> 	@0x103
> user32.dll	NtUserPeekMessage
> user32.dll	_PeekMessage
> xul.dll	mozilla::widget::WinUtils::PeekMessageW(tagMSG *,HWND__ *,unsigned int,unsigned int,unsigned int)
> xul.dll	mozilla::widget::NativeKey::IsFollowedByDeadCharMessage()
> xul.dll	mozilla::widget::NativeKey::NativeKey(nsWindowBase *,tagMSG const &,mozilla::widget::ModifierKeyState const &,nsTArray<mozilla::widget::NativeKey::FakeCharMsg> *)
> xul.dll	nsWindow::ProcessKeyDownMessage(tagMSG const &,bool *)
> xul.dll	nsWindow::ProcessMessage(unsigned int,unsigned int &,long &,long *)
> xul.dll	nsWindow::WindowProcInternal(HWND__ *,unsigned int,unsigned int,long)
> xul.dll	CallWindowProcCrashProtected
> xul.dll	nsWindow::WindowProc(HWND__ *,unsigned int,unsigned int,long)
> user32.dll	InternalCallWinProc
> user32.dll	UserCallWinProcCheckWow
> user32.dll	SendMessageWorker
> user32.dll	SendMessageW
> pgdll.dll	pgdll.dll@0x30f8
> 	@0x103
> user32.dll	NtUserPeekMessage
> user32.dll	_PeekMessage
> xul.dll	mozilla::widget::WinUtils::PeekMessageW(tagMSG *,HWND__ *,unsigned int,unsigned int,unsigned int)
> xul.dll	mozilla::widget::NativeKey::IsFollowedByDeadCharMessage()
> xul.dll	mozilla::widget::NativeKey::NativeKey(nsWindowBase *,tagMSG const &,mozilla::widget::ModifierKeyState const &,nsTArray<mozilla::widget::NativeKey::FakeCharMsg> *)
> xul.dll	nsWindow::ProcessKeyDownMessage(tagMSG const &,bool *)
> xul.dll	nsWindow::ProcessMessage(unsigned int,unsigned int &,long &,long *)
> xul.dll	nsWindow::WindowProcInternal(HWND__ *,unsigned int,unsigned int,long)
> xul.dll	CallWindowProcCrashProtected
> xul.dll	nsWindow::WindowProc(HWND__ *,unsigned int,unsigned int,long)
> user32.dll	InternalCallWinProc
> user32.dll	UserCallWinProcCheckWow
> user32.dll	SendMessageWorker
> xul.dll	nsBaseAppShell::Run()
> xul.dll	nsAppShell::Run()
> nss3.dll	nss3.dll@0x7960
> xul.dll	XREMain::XRE_main(int,char * * const,nsXREAppData const *)
> xul.dll	XRE_main
> firefox.exe	do_main
> firefox.exe	NS_internal_main(int,char * *)

pgdll.dll has API hook or something which makes nested key message loop at calling PeekMessage() API.

I.e., we should make NativeKey detect such nested case and avoid crash at that time.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Crash volume for signature 'mozilla::widget::NativeKey::GetFollowingCharMessage':
 - nightly (version 51): 0 crashes from 2016-08-01.
 - aurora  (version 50): 0 crashes from 2016-08-01.
 - beta    (version 49): 41 crashes from 2016-08-02.
 - release (version 48): 31 crashes from 2016-07-25.
 - esr     (version 45): 16 crashes from 2016-05-02.

Crash volume on the last weeks (Week N is from 08-22 to 08-28):
            W. N-1  W. N-2  W. N-3
 - nightly       0       0       0
 - aurora        0       0       0
 - beta         14      12       1
 - release      14       5       5
 - esr           5       2       2

Affected platform: Windows

Crash rank on the last 7 days:
           Browser     Content   Plugin
 - nightly
 - aurora
 - beta    #718
 - release #1652
 - esr     #1213

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Perhaps, this is fixed by the fix of bug 1302956. However, we need to keep watching the crash reports of 52 or later because most victims don't use Nightly (nor perhaps Aurora).

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

According to the current reports, I don't see this crash (caused by nested message handling). I think that this was fixed by bug 1302956.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Crash volume for signature 'mozilla::widget::NativeKey::GetFollowingCharMessage':
 - nightly (version 54): 0 crashes from 2017-01-23.
 - aurora  (version 53): 0 crashes from 2017-01-23.
 - beta    (version 52): 258 crashes from 2017-01-23.
 - release (version 51): 13 crashes from 2017-01-16.
 - esr     (version 45): 68 crashes from 2016-08-03.

Crash volume on the last weeks (Week N is from 01-30 to 02-05):
            W. N-1  W. N-2  W. N-3  W. N-4  W. N-5  W. N-6  W. N-7
 - nightly       0
 - aurora        0
 - beta         95
 - release       7       0
 - esr           4       3       3       0       0       2       4

Affected platform: Windows

Crash rank on the last 7 days:
           Browser   Content   Plugin
 - nightly
 - aurora
 - beta    #27
 - release #1358
 - esr     #1921

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Still seeing this signature on Fx52 beta builds :(

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

No, this is just for the crash caused by nested message handling. As far as I see new crash reports, there are no crash reports caused by that.

Please watch bug 962140 for same crash signature but caused by other reasons. I'm working on fixing each case right now.