952116 - crash in mozilla::TimeDuration::operator>(mozilla::TimeDuration const&)

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect)

Description

[Jim Mathies [:jimm]]

0 	xul.dll 	mozilla::TimeDuration::operator>(mozilla::TimeDuration const &) 	obj-firefox/dist/include/mozilla/TimeStamp.h
1 	xul.dll 	mozilla::layers::AsyncPanZoomController::UpdateAnimation(mozilla::TimeStamp const &) 	gfx/layers/ipc/AsyncPanZoomController.cpp
2 	xul.dll 	mozilla::layers::AsyncPanZoomController::SampleContentTransformForFrame(mozilla::TimeStamp const &,mozilla::layers::ViewTransform *,mozilla::gfx::PointTyped<mozilla::ScreenPixel> &) 	gfx/layers/ipc/AsyncPanZoomController.cpp
3 	xul.dll 	mozilla::layers::AsyncCompositionManager::ApplyAsyncContentTransformToTree(mozilla::TimeStamp,mozilla::layers::Layer *,bool *) 	gfx/layers/composite/AsyncCompositionManager.cpp
4 	xul.dll 	mozilla::layers::AsyncCompositionManager::ApplyAsyncContentTransformToTree(mozilla::TimeStamp,mozilla::layers::Layer *,bool *) 	gfx/layers/composite/AsyncCompositionManager.cpp
5 	xul.dll 	mozilla::layers::AsyncCompositionManager::ApplyAsyncContentTransformToTree(mozilla::TimeStamp,mozilla::layers::Layer *,bool *) 	gfx/layers/composite/AsyncCompositionManager.cpp
6 	xul.dll 	mozilla::layers::AsyncCompositionManager::TransformShadowTree(mozilla::TimeStamp) 	gfx/layers/composite/AsyncCompositionManager.cpp
7 	xul.dll 	mozilla::layers::CompositorParent::CompositeInTransaction() 	gfx/layers/ipc/CompositorParent.cpp
8 	xul.dll 	mozilla::layers::CompositorParent::Composite() 	gfx/layers/ipc/CompositorParent.cpp
9 	xul.dll 	MessageLoop::RunTask(Task *) 	ipc/chromium/src/base/message_loop.cc
10 	xul.dll 	MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const &) 	ipc/chromium/src/base/message_loop.cc
11 	xul.dll 	MessageLoop::DoDelayedWork(base::TimeTicks *) 	ipc/chromium/src/base/message_loop.cc
12 	xul.dll 	base::MessagePumpDefault::Run(base::MessagePump::Delegate *) 	ipc/chromium/src/base/message_pump_default.cc
13 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
14 	xul.dll 	MessageLoop::Run()

This bug was filed from the Socorro interface and is 
report bp-091217d4-41e2-41a9-b539-424602131216.
=============================================================

Comment 1

[Jim Mathies [:jimm]]

https://crash-stats.mozilla.com/report/list?product=MetroFirefox&range_value=7&range_unit=days&date=2013-12-19&signature=mozilla%3A%3ATimeDuration%3A%3Aoperator%3E%28mozilla%3A%3ATimeDuration+const%26%29&version=MetroFirefox%3A29.0a1

Comment 2

[Kartikaya Gupta (email:[e-mail])]

mAnimation can be nulled out on the input-receiving thread (i.e. the gecko thread) while the compositor thread is trying to query it via SampleContentTransformForFrame.

Anthony, do you have some cycles to look into this?

Comment 3

[Kartikaya Gupta (email:[e-mail])]

(I suspect all that needs to be done here is wrap the body CancelAnimation() inside a mMonitor lock, but it would be nice to have STR and verify the fix)

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

mAnimation is only being accessed in StartAnimation, CancelAnimation and UpdateAnimation. CancelAnimation is missing a mutex so I'll just add one.

Comment 5

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Add mutex around APZC::CancelAnimation;

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8350240 [details] [diff] [review]
Add mutex around APZC::CancelAnimation;

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

At some point we need to properly lock the state changes too. But that's a different bug for a different day.

Comment 7

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/de90cb114d46

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 9

[Matt Brubeck (:mbrubeck)]

It looks like this crash has shown up in Firefox 28 too.  Should we request Aurora approval?

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8350240 [details] [diff] [review]
Add mutex around APZC::CancelAnimation;

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 839911
User impact if declined: occasional crashes
Testing completed (on m-c, etc.): fix is unconfirmed as of yet
Risk to taking this patch (and alternatives if risky): pretty low risk. even if this patch doesn't fix the crash it fixes an obvious bug in the code
String or IDL/UUID changes made by this patch: none

Comment 11

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/releases/mozilla-aurora/rev/39533c25c43c

Comment 13

[u279076]

Marking this verified fixed since I'm only seeing 9 reports of this crash in the last week, none of which are in Firefox 28:
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3ATimeDuration%3A%3Aoperator%3E%28mozilla%3A%3ATimeDuration+const%26%29&product=Firefox&query_type=contains&range_unit=weeks&process_type=any&hang_type=any&date=2014-01-07+19%3A00%3A00&range_value=1#reports