Closed Bug 866952 Opened 7 years ago Closed 7 years ago

Camera and camcorder conspire to crash B2G

Categories

(Core :: Graphics: Layers, defect, critical)

x86
Gonk (Firefox OS)
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: milan, Assigned: bas.schouten)

References

Details

(Keywords: crash, Whiteboard: [b2g-crash])

Crash Data

Attachments

(1 file)

On the trunk; go to camera, take a photo, switch to camcorder, crash.
I certainly couldn't reproduce a crash as easily as it is described here. But after taking a bunch of pictures, recording some videos and taking pictures again (with the patch of bug 866521 applied), I crashed with the following stack:

#0  0x41c5bb48 in mozilla::layers::GrallocBufferActor::ActorDestroy (this=0x486a1ce0)
    at /home/bas/Dev/mozilla-central/gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp:230
#1  0x4182dbae in mozilla::plugins::PPluginBackgroundDestroyerParent::DestroySubtree (
    this=0x486a1cfc, 
    why=mozilla::ipc::IProtocolManager<mozilla::ipc::RPCChannel::RPCListener>::Deletion)
    at /home/bas/Dev/mozilla-central/objdir-gonk/ipc/ipdl/PPluginBackgroundDestroyerParent.cpp:332
#2  0x41877e62 in mozilla::layers::PGrallocBufferParent::OnMessageReceived (
    this=0x486a1cfc, __msg=...)
    at /home/bas/Dev/mozilla-central/objdir-gonk/ipc/ipdl/PGrallocBufferParent.cpp:209
#3  0x4187b736 in mozilla::layers::PImageBridgeParent::OnMessageReceived (
    this=0x4a5bd8e0, __msg=...)
    at /home/bas/Dev/mozilla-central/objdir-gonk/ipc/ipdl/PImageBridgeParent.cpp:404
#4  0x4180c12a in mozilla::ipc::AsyncChannel::OnDispatchMessage (this=0x4a5bd8f0, 
    msg=...) at /home/bas/Dev/mozilla-central/ipc/glue/AsyncChannel.cpp:492
#5  0x41812e58 in mozilla::ipc::RPCChannel::OnMaybeDequeueOne (this=0x4a5bd8f0)
    at /home/bas/Dev/mozilla-central/ipc/glue/RPCChannel.cpp:402
#6  0x417de86c in DispatchToMethod<mozilla::dom::ContentParent, void (mozilla::dom::ContentParent::*)()> (this=<value optimized out>)
    at /home/bas/Dev/mozilla-central/ipc/chromium/src/base/tuple.h:383
#7  RunnableMethod<mozilla::dom::ContentParent, void (mozilla::dom::ContentParent::*)(), Tuple0>::Run (this=<value optimized out>)
    at /home/bas/Dev/mozilla-central/ipc/chromium/src/base/task.h:307
#8  0x4181129a in mozilla::ipc::RPCChannel::RefCountedTask::Run (this=0x483391a0)
(gdb) p *mTextureHost
$3 = {<mozilla::layers::TextureSource> = {<mozilla::RefCounted<mozilla::layers::TextureSource>> = {refCnt = 1515870810}, _vptr.TextureSource = 0x5a5a5a5a}, mFlags = 1515870810, 
  mBuffer = 0x5a5a5a5a, mFormat = 1515870810, mDeAllocator = 0x5a5a5a5a}

I'm guessing this is a use after free, maybe this was fixed by Benoit's patches? I'm not sure, this was based off central for changeset 05533d50f2f7.
Severity: normal → critical
Crash Signature: [@ mozilla::layers::GrallocBufferActor::ActorDestroy()]
Keywords: crash
Whiteboard: [b2g-crash]
Assignee: nobody → bas
Status: NEW → ASSIGNED
Attachment #743597 - Flags: review?(bjacob)
Comment on attachment 743597 [details] [diff] [review]
Unregister from old GrallocBufferActor when switching buffers.

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

r=me with nits. Congrats of the awesome debugging (Bas confirmed on IRC that we're not getting double Send__delete__, instead we're being tricked by ImageHost's way of double buffering).

Also, please make patches with 8 lines of context and function names.

::: gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp
@@ +175,4 @@
>  
>  GrallocBufferActor::GrallocBufferActor()
>  : mAllocBytes(0)
> +, mTextureHost(nullptr)

It's a good idea to move this initialization to the initializer list, but please remove the

  mTextureHost = nullptr;

a few lines down.

::: gfx/layers/opengl/TextureHostOGL.cpp
@@ +691,4 @@
>                                          nsIntRegion*)
>  {
>    MOZ_ASSERT(aImage.type() == SurfaceDescriptor::TSurfaceDescriptorGralloc);
> +  

trailing whitespace

@@ +693,5 @@
>    MOZ_ASSERT(aImage.type() == SurfaceDescriptor::TSurfaceDescriptorGralloc);
> +  
> +  if (mBuffer) {
> +    RegisterTextureHostAtGrallocBufferActor(nullptr, *mBuffer);
> +  }

Hah. Great catch. The ImageHost's funky way of doing double buffering kicking us in the...
Attachment #743597 - Flags: review?(bjacob) → review+
Oh and can you please add

// only used for hacky fix in gecko 23 for bug 862324

where you call RegisterTextureHostAtGrallocBufferActor(nullptr, *mBuffer);
https://hg.mozilla.org/mozilla-central/rev/aa4a8b56c143
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.