PeekMessage(PM_REMOVE) sometimes fails to remove found char message (returns false) even if the message still in the queue

RESOLVED FIXED in Firefox 29

Status

()

defect
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({crash, topcrash-metro})

Trunk
mozilla30
x86_64
Windows 8.1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox28- wontfix, firefox29- fixed, firefox30- fixed)

Details

(Whiteboard: "PeekMessage() failed to remove char message!" in the Notes of crash reports and "Next key message in all windows" is same message as "Found message", crash signature)

Attachments

(1 attachment)

https://crash-stats.mozilla.com/report/index/d2cd4736-3fa9-4736-a8c8-b86ef2140310
https://crash-stats.mozilla.com/report/index/e8165a55-6747-4b10-8a1b-df5c82140310

> Handling message: WM_KEYDOWN (0x00000100), wParam: 0x00000008, lParam: 0x000E0001, hwnd=0x1500b6, InSendMessageEx()=ISMEX_NOSEND, 
> Found message: WM_DEADCHAR (0x00000103), wParam: 0x00000020, lParam: 0x000E0001, hwnd=0x1500b6, 
> WM_NULL has been removed: 0, 
> Next key message in all windows: WM_DEADCHAR (0x00000103), wParam: 0x00000020, lParam: 0x000E0001, hwnd=0x1500b6, time=7724890, 
> Next message in all windows: WM_DEADCHAR (0x00000103), wParam: 0x00000020, lParam: 0x000E0001, hwnd=0x1500b6, time=7724890

> Handling message: WM_KEYDOWN (0x00000100), wParam: 0x00000008, lParam: 0x000E0001, hwnd=0x1202e0, InSendMessageEx()=ISMEX_NOSEND, 
> Found message: WM_DEADCHAR (0x00000103), wParam: 0x00000020, lParam: 0x000E0001, hwnd=0x1202e0, 
> WM_NULL has been removed: 0, 
> Next key message in all windows: WM_DEADCHAR (0x00000103), wParam: 0x00000020, lParam: 0x000E0001, hwnd=0x1202e0, time=18573562, 
> Next message in all windows: WM_DEADCHAR (0x00000103), wParam: 0x00000020, lParam: 0x000E0001, hwnd=0x1202e0, time=18573562

This is very wired. PeekMessage() fails to remove found char message which still exists in the queue...
https://crash-stats.mozilla.com/report/index/a03ed936-3cba-4b56-83f5-3828f2140309

> Handling message: WM_SYSKEYDOWN (0x00000104), wParam: 0x00000035, lParam: 0x20060001, hwnd=0x106f2, InSendMessageEx()=ISMEX_NOSEND, 
> Found message: WM_CHAR (0x00000102), wParam: 0x000000BD, lParam: 0x20060001, hwnd=0x106f2, 
> WM_NULL has been removed: 0, 
> Next key message in all windows: WM_CHAR (0x00000102), wParam: 0x000000BD, lParam: 0x20060001, hwnd=0x106f2, time=120333875, 
> Next message in all windows: WM_CHAR (0x00000102), wParam: 0x000000BD, lParam: 0x20060001, hwnd=0x106f2, time=120333875

This is a WM_CHAR case.
https://crash-stats.mozilla.com/report/index/34c5bb82-77f7-4e77-8c9f-2108c2140310

> Handling message: WM_SYSKEYDOWN (0x00000104), wParam: 0x0000000D, lParam: 0x201C0001, hwnd=0x102f4, InSendMessageEx()=ISMEX_NOSEND, 
> Found message: WM_CHAR (0x00000102), wParam: 0x00000000, lParam: 0x201C0001, hwnd=0x102f4, 
> WM_NULL has been removed: 0, 
> Next key message in all windows: WM_CHAR (0x00000102), wParam: 0x00000000, lParam: 0x201C0001, hwnd=0x102f4, time=1520703, 
> Next message in all windows: WM_CHAR (0x00000102), wParam: 0x00000000, lParam: 0x201C0001, hwnd=0x102f4, time=1520703

This is very rare. The wParam (input character) is a null character.
Oh, I realized that these crashes occurred GetFollowingCharMsg() is called by DispatchPluginEventsAndDiscardsCharMessages().
This is really experimental patch.

When the message queue has the found char message but PeekMessage() with PM_REMOVE fails to remove it, let's retry with GetMessage().
Attachment #8389496 - Flags: review?(jmathies)
I think that if the patch cannot fix this bug and we don't find other approaches for this, we should not dispatch keypress event in this case because the char message must come later.
Attachment #8389496 - Flags: review?(jmathies) → review+
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #6)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/6df586c17279
> 
> We need to uplift this since no Nightly testers report this crash :-(

Is this showing up on desktop?
(In reply to Jim Mathies [:jimm] (away 4/1-4/21) from comment #7)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #6)
> > https://hg.mozilla.org/integration/mozilla-inbound/rev/6df586c17279
> > 
> > We need to uplift this since no Nightly testers report this crash :-(
> 
> Is this showing up on desktop?

Yes.
As this is a Metro topcrash and we're not shipping Metro yet in FF28, we will not need to track this one.
(In reply to Lukas Blakk [:lsblakk] from comment #10)
> As this is a Metro topcrash and we're not shipping Metro yet in FF28, we
> will not need to track this one.

Yeah, but the crash also occurs on desktop Firefox. So, when I request uplift the patch(es), is it possible to be allowed? Unfortunately, no Nightly testers report this crash. So, we need to uplift the experimental patch and if it'll fail to fix this bug, we should land next patch which completely avoids the crash in this situation.
Flags: needinfo?(lsblakk)
I mean, not for 28, it's too late. I think that we should fix this bug at least on 29.
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #12)
> I mean, not for 28, it's too late. I think that we should fix this bug at
> least on 29.

Yes, nominate for uplift to Aurora if it's a  low risk landing.
Flags: needinfo?(lsblakk)
Comment on attachment 8389496 [details] [diff] [review]
Retry to remove found char message with GetMessage() when PeekMessage() fails to remove the message from the queue

[Approval Request Comment]
User impact if declined: If we're lucky, this patch can fix this bug. The cause of this bug is, Win32 API's undocumented behavior. This patch retry with another API when we meet the unexpected situation. Unluckily, we have no Nightly testers who can reproduce this bug. We need to uplift this patch to check if this patch can fix this bug.
Testing completed (on m-c, etc.): Landed on m-c.
Risk to taking this patch (and alternatives if risky): The risk must be low. This change doesn't affect to normal path.
String or IDL/UUID changes made by this patch: No.
Attachment #8389496 - Flags: approval-mozilla-aurora?
Attachment #8389496 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
FYI: It seems that only one tester meets this crash on Aurora. The latest crash report takes 5 days from previous report. I think that we need to watch a couple of weeks if it's actually fixed. Fortunately, Aurora becomes Beta next week. So, we probably see new crash report in next week if it's not fixed.
Whiteboard: [leave open] → [leave open] "PeekMessage() failed to remove char message!" in the Notes of crash reports
Very interesting. The patch improves the crash frequency actually. However, there are following crash reports:

https://crash-stats.mozilla.com/report/index/67ffc2e6-9768-466e-9fd5-4eb202140322
> PeekMessage() failed to remove char message! 
> Handling message: WM_KEYDOWN (0x00000100), wParam: 0x0000000D, lParam: 0x001C0001, hwnd=0x1301e8, InSendMessageEx()=ISMEX_NOSEND, 
> Found message: WM_CHAR (0x00000102), wParam: 0x0000000D, lParam: 0x001C0001, hwnd=0x1301e8, 
> WM_NULL has been removed: 1, 
> Next key message in all windows: WM_KEYUP (0x00000101), wParam: 0x0000000D, lParam: 0xC01C0001, hwnd=0x1301e8, time=152910656, 
> Next message in all windows: Unknown (0x0000C1B3), wParam: 0x00000000, lParam: 0x070FAB00, hwnd=0x1703c8, time=152910734

https://crash-stats.mozilla.com/report/index/d1246ae5-6d8f-41ee-850f-395d92140324
> PeekMessage() failed to remove char message! 
> Handling message: WM_KEYDOWN (0x00000100), wParam: 0x00000020, lParam: 0x00390001, hwnd=0x30320, InSendMessageEx()=ISMEX_NOSEND, 
> Found message: WM_CHAR (0x00000102), wParam: 0x00000020, lParam: 0x00390001, hwnd=0x30320, 
> WM_NULL has been removed: 1, 
> Next key message in all windows: WM_KEYUP (0x00000101), wParam: 0x00000020, lParam: 0xC0390001, hwnd=0x30320, time=7563968, 
> Next message in all windows: Unknown (0x0000C183), wParam: 0x00000000, lParam: 0x065FAAB0, hwnd=0x902f4, time=7563968

In these reports, the found WM_CHAR message are really lost.

So, this bug has been fixed by the patch. But we need more work for the remaining case.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [leave open] "PeekMessage() failed to remove char message!" in the Notes of crash reports → "PeekMessage() failed to remove char message!" in the Notes of crash reports and "Next key message in all windows" is same message as "Found message"
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.