Crash in mozilla::ContentCacheInParent::FlushPendingNotifications

RESOLVED FIXED in Firefox 55

Status

()

--
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: philipp, Assigned: masayuki)

Tracking

({crash, regression})

56 Branch
mozilla56
crash, regression
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 unaffected, firefox55 fixed, firefox56 fixed)

Details

(crash signature)

Attachments

(2 attachments)

(Reporter)

Description

a year ago
This bug was filed from the Socorro interface and is 
report bp-bd9a8d05-413e-4fc9-b799-83f8f0170705.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::ContentCacheInParent::FlushPendingNotifications(nsIWidget*) 	widget/ContentCache.cpp:1350
1 	xul.dll 	mozilla::ContentCacheInParent::OnEventNeedingAckHandled(nsIWidget*, mozilla::EventMessage) 	widget/ContentCache.cpp:1210
2 	xul.dll 	mozilla::dom::TabParent::RecvOnEventNeedingAckHandled(mozilla::EventMessage const&) 	dom/ipc/TabParent.cpp:1819
3 	xul.dll 	mozilla::dom::PBrowserParent::OnMessageReceived(IPC::Message const&) 	obj-firefox/ipc/ipdl/PBrowserParent.cpp:2256
4 	xul.dll 	mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) 	obj-firefox/ipc/ipdl/PContentParent.cpp:3203
5 	xul.dll 	mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) 	ipc/glue/MessageChannel.cpp:2093
6 	xul.dll 	mozilla::ipc::MessageChannel::DispatchMessageW(IPC::Message&&) 	ipc/glue/MessageChannel.cpp:2019
7 	xul.dll 	mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) 	ipc/glue/MessageChannel.cpp:1888
8 	xul.dll 	mozilla::ipc::MessageChannel::MessageTask::Run() 	ipc/glue/MessageChannel.cpp:1921
9 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1422
10 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:97
11 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:313
12 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:293
13 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp:156
14 	xul.dll 	nsAppShell::Run() 	widget/windows/nsAppShell.cpp:271
15 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/nsAppStartup.cpp:287
16 	xul.dll 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp:4589
17 	xul.dll 	XREMain::XRE_main(int, char** const, mozilla::BootstrapConfig const&) 	toolkit/xre/nsAppRunner.cpp:4772
18 	xul.dll 	XRE_main(int, char** const, mozilla::BootstrapConfig const&) 	toolkit/xre/nsAppRunner.cpp:4867
19 	firefox.exe 	NS_internal_main(int, char**, char**) 	browser/app/nsBrowserApp.cpp:310
20 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:115
21 	firefox.exe 	__scrt_common_main_seh 	f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:253
22 	kernel32.dll 	BaseThreadInitThunk 	
23 	ntdll.dll 	RtlUserThreadStart

these crashes are showing up a handful of times on 56 nightly - the first occurrence was with build 20170703030203.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Blocks: 1376424
Keywords: regression
Comment hidden (mozreview-request)

Comment 3

a year ago
mozreview-review
Comment on attachment 8885123 [details]
Bug 1379448 - ContentCacheInParent::FlushPendingNotifications() should do nothing if aWidget is nullptr

https://reviewboard.mozilla.org/r/155974/#review161264
Attachment #8885123 - Flags: review?(m_kato) → review+

Comment 4

a year ago
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/d95e9e0676a0
ContentCacheInParent::FlushPendingNotifications() should do nothing if aWidget is nullptr r=m_kato

Comment 5

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d95e9e0676a0
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox56: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
status-firefox55: unaffected → affected
Created attachment 8887450 [details] [diff] [review]
Patch for Beta

Approval Request Comment
[Feature/Bug causing the regression]:
Regression of bug 1376424.

[User impact if declined]:
May meet this crash if widget for TabParent has gone when it receives notification or request to IME from a remote process.

[Is this code covered by automated tests?]:
No.

[Has the fix been verified in Nightly?]:
Yes, there is no new crash report:
https://crash-stats.mozilla.com/signature/?product=Firefox&version=56.0a1&signature=mozilla%3A%3AContentCacheInParent%3A%3AFlushPendingNotifications&date=%3E%3D2017-07-04T12%3A11%3A19.000Z&date=%3C2017-07-18T12%3A11%3A19.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-date&page=1#reports

[Needs manual test from QE? If yes, steps to reproduce]: 
No.

[List of other uplifts needed for the feature/fix]:
No.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
Just adding new nullptr check before using widget.

[String changes made/needed]:
No.
Attachment #8887450 - Flags: review+
Attachment #8887450 - Flags: approval-mozilla-beta?
Comment on attachment 8887450 [details] [diff] [review]
Patch for Beta

fix a crash introduced in beta last week, beta55+
Attachment #8887450 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 8

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/4503ab07264c
status-firefox55: affected → fixed
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #6)
> [Is this code covered by automated tests?]:
> No.
> 
> [Has the fix been verified in Nightly?]:
> Yes, there is no new crash report:
> https://crash-stats.mozilla.com/signature/?product=Firefox&version=56.
> 0a1&signature=mozilla%3A%3AContentCacheInParent%3A%3AFlushPendingNotification
> s&date=%3E%3D2017-07-04T12%3A11%3A19.000Z&date=%3C2017-07-18T12%3A11%3A19.
> 000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_colum
> ns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-
> date&page=1#reports
> 
> [Needs manual test from QE? If yes, steps to reproduce]: 
> No.

Setting qe-verify- based on Masayuki Nakano's assessment on manual testing needs.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.