868741 - crash in ParticularProcessPriorityManager::ComputePriority

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Scoobidiver (away)]

It has been hit by three users since B2G 18.0/20130501230205 and one since B2G 23.0a1/20130501085824. Stack traces differ between 18.0 and 23.0a1.

Here is a crash report in B2G 18.0: bp-891c1d4a-ce32-4906-a5b1-6ef4e2130503.

Frame 	Module 	Signature 	Source
0 	libxul.so 	ParticularProcessPriorityManager::ComputePriority 	nsTArray.h:203
1 	libxul.so 	ParticularProcessPriorityManager::ResetPriority 	ProcessPriorityManager.cpp:651
2 	libxul.so 	ParticularProcessPriorityManager::Notify 	ProcessPriorityManager.cpp:506
3 	libxul.so 	mozilla::hal::NotifyWakeLockChange 	Observer.h:67
4 	libxul.so 	mozilla::hal_impl::ModifyWakeLock 	HalWakeLock.cpp:239
5 	libxul.so 	mozilla::hal::ModifyWakeLock 	Hal.cpp:651
6 	libxul.so 	mozilla::dom::power::WakeLock::HandleEvent 	WakeLock.cpp:257
7 	libxul.so 	nsEventListenerManager::HandleEventSubType 	nsEventListenerManager.cpp:889
8 	libxul.so 	nsEventListenerManager::HandleEventInternal 	nsEventListenerManager.cpp:962
9 	libxul.so 	nsEventTargetChainItem::HandleEvent 	nsEventListenerManager.h:144
10 	libxul.so 	nsEventTargetChainItem::HandleEventTargetChain 	nsEventDispatcher.cpp:316
11 	libxul.so 	nsEventTargetChainItem::HandleEventTargetChain 	nsEventDispatcher.cpp:371
12 	libxul.so 	nsEventDispatcher::Dispatch 	nsEventDispatcher.cpp:634
13 	libxul.so 	nsEventDispatcher::DispatchDOMEvent 	nsEventDispatcher.cpp:694
14 	libxul.so 	nsINode::DispatchEvent 	nsINode.cpp:1078
15 	libxul.so 	nsContentUtils::DispatchEvent 	nsContentUtils.cpp:3547
16 	libxul.so 	nsContentUtils::DispatchTrustedEvent 	nsContentUtils.cpp:3518
17 	libxul.so 	nsDocument::UpdateVisibilityState 	nsDocument.cpp:9943
18 	libxul.so 	nsRunnableMethodImpl<nsrefcnt , false>::Run 	nsThreadUtils.h:366
19 	libxul.so 	nsThread::ProcessNextEvent 	nsThread.cpp:620
20 	libxul.so 	NS_ProcessNextEvent_P 	nsThreadUtils.cpp:237
21 	libxul.so 	mozilla::ipc::MessagePump::Run 	MessagePump.cpp:82
22 	libxul.so 	MessageLoop::RunInternal 	message_loop.cc:219
23 	libxul.so 	MessageLoop::Run 	message_loop.cc:212
24 	libxul.so 	nsBaseAppShell::Run 	nsBaseAppShell.cpp:163
25 	libxul.so 	nsAppStartup::Run 	nsAppStartup.cpp:290
26 	libxul.so 	XREMain::XRE_mainRun 	nsAppRunner.cpp:3794
27 	libxul.so 	XREMain::XRE_main 	nsAppRunner.cpp:3860
28 	libxul.so 	XRE_main 	nsAppRunner.cpp:3935
29 	b2g 	main 	nsBrowserApp.cpp:168
30 	libc.so 	__libc_init 	libc_init_dynamic.c:114
31 	libc.so 	__cxa_atexit 	atexit.c:99
32 		@0xbec01d3c 	

More reports at:
https://crash-stats.mozilla.com/report/list?signature=ParticularProcessPriorityManager%3A%3AComputePriority

Comment 1

[Michael Vines [:m1] [:evilmachines]]

(leo+, this is showing up as a part of stability test)

Comment 3

[Jason Smith [:jsmith]]

Reproducible end-user STR is in the dupe of this bug.

Comment 4

[Jason Smith [:jsmith]]

Crashing in this file: https://hg.mozilla.org/mozilla-central/file/b842d26dd5f0/dom/ipc/ProcessPriorityManager.cpp

Comment 5

[Justin Lebar (not reading bugmail)]

This is crashing on nsTArray::Length, so

https://hg.mozilla.org/mozilla-central/file/b842d26dd5f0/dom/ipc/ProcessPriorityManager.cpp#l739

This probably means that mContentParent is a dangling pointer.

Comment 6

[Justin Lebar (not reading bugmail)]

Marked as security-sensitive because I'm afraid this is a use-after-free.  We can unprotect it if I'm wrong.

Comment 7

[Jason Smith [:jsmith]]

Hmm...is this a regression from bug 844323 by any chance? If so, this should be nominated to tef?

Comment 8

[Justin Lebar (not reading bugmail)]

It's definitely a regression from that bug.  But I don't see why that should affect whether or not it blocks tef.

Comment 9

[Jason Smith [:jsmith]]

(In reply to Justin Lebar [:jlebar] from comment #8)
> It's definitely a regression from that bug.  But I don't see why that should
> affect whether or not it blocks tef.

My original reasoning is that this is 1.01 affected and a crash regression from a tef+ blocker that's reproducible and security-sensitive. Unless there's some argument that states that we should block on this for 1.1, but not 1.01 that I'm not seeing here.

Do you have any concerns on blocking on this for 1.01?

Comment 10

[Justin Lebar (not reading bugmail)]

> Do you have any concerns on blocking on this for 1.01?

Definitely not.  My only point is that a crash that affects 1.0.1 and is reproducible and s-s is bad, regardless of how it was introduced.

Comment 11

[Justin Lebar (not reading bugmail)]

Oh goodness, this is just a null-pointer exception.  I should have looked more closely at the crash report:

> Crash Address 0x168

In other words, null plus change.

Comment 12

[Daniel Holbert [:dholbert]]

Un-marking as security-sensitive, per jlebar's request in comment 11 & in IRC (and since he's the one who initially requested that it be hidden, back in Comment 6).

Comment 13

[Daniel Holbert [:dholbert]]

s/jlebar's request in comment 11 & in IRC/jlebar's point in comment 11 & request in IRC/

Comment 14

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v1

Comment 15

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 746595 [details] [diff] [review]
Patch, v1

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

LGTM!

Comment 16

[Justin Lebar (not reading bugmail)]

This is a safe crash-fix patch that we should take on tef.

Comment 17

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/projects/birch/rev/e410391c052f

Comment 18

[Scoobidiver (away)]

Naoki, the use of the topcrash keyword for B2G should be documented in https://wiki.mozilla.org/CrashKill/Topcrash.

Comment 19

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I hit this crash twice today already on leo.  Please uplift.

Comment 20

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

(In reply to Scoobidiver from comment #18)
> Naoki, the use of the topcrash keyword for B2G should be documented in
> https://wiki.mozilla.org/CrashKill/Topcrash.

Done.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

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

Comment 22

[Michael Vines [:m1] [:evilmachines]]

Ryan, can you please uplift to v1-train while the v1.0.1 folks consider the tef? request.  leo+ was set @ comment 1

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Literally right after I push a bunch of other stuff to b2g18, of course...

https://hg.mozilla.org/releases/mozilla-b2g18/rev/f1fdd7d57305

Comment 25

[Justin Lebar (not reading bugmail)]

This was tef?'ed six days ago, well before the most recent deadline.

How often is tef triage happening these days?  Surely when a deadline is coming up, we should clear our blocker backlog so that important patches don't miss the boat.

If the delay here means that this misses 1.0.1, I'll be pretty disappointed.  But even if not, the delay means that taking this patch is more risky, since it'll get less testing (although I still think we're better off taking this patch than not).  Triage is supposed to decrease our risk; something is very wrong when development becomes more risky due to our triage practices.

Comment 26

[Daniel Coloma:dcoloma]

(In reply to Justin Lebar [:jlebar] from comment #25)
> This was tef?'ed six days ago, well before the most recent deadline.
> 
> How often is tef triage happening these days?  Surely when a deadline is
> coming up, we should clear our blocker backlog so that important patches
> don't miss the boat.
> 
> If the delay here means that this misses 1.0.1, I'll be pretty disappointed.
> But even if not, the delay means that taking this patch is more risky, since
> it'll get less testing (although I still think we're better off taking this
> patch than not).  Triage is supposed to decrease our risk; something is very
> wrong when development becomes more risky due to our triage practices.

Sorry Justin, somehow I missed this bug, I've marked as tef+

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/3fd9aa7324a1