831167 - crash in mozilla::dom::devicestorage::PDeviceStorageRequestParent::Write

Status: VERIFIED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

Attachment: decoded minidump of crash

Seen during camcorder.py stability test run using AU 176.  

.extra file contains:
xpcom_runtime_abort([Parent 127] ###!!! ABORT: NULL actor value passed to non-nullable param: file /local/mnt/workspace/lnxbuild/project/release_dev_msm7627a_616740/checkout/out/target/product/msm7627a/obj/objdir-gecko/ipc/ipdl/PDeviceStorageRequestParent.cpp, line 634)

Top frames:
Thread 0 (crashed)
 0  libxul.so!mozalloc_abort [mozalloc_abort.cpp : 30 + 0x4]
     r4 = 0xbeeef6d4    r5 = 0x00000000    r6 = 0xffffffff    r7 = 0xbeeef2e8
     r8 = 0x41153219    r9 = 0x00000001   r10 = 0xbeeef2e8    fp = 0x4163958d
     sp = 0xbeeef2d0    lr = 0x415c95df    pc = 0x415c95e2
    Found by: given as instruction pointer in context
 1  libxul.so!NS_DebugBreak_P [nsDebugImpl.cpp : 423 + 0x5]
     r4 = 0xbeeef6d4    r5 = 0x00000000    r6 = 0xffffffff    r7 = 0xbeeef2e8
     r8 = 0x41153219    r9 = 0x00000001   r10 = 0xbeeef2e8    fp = 0x4163958d
     sp = 0xbeeef2d8    pc = 0x41153001
    Found by: call frame info
 2  libxul.so!mozilla::dom::devicestorage::PDeviceStorageRequestParent::Write [PDeviceStorageRequestParent.cpp : 641 + 0x15]
     r4 = 0x00000000    r5 = 0x441beb00    r6 = 0x45477ee0    r7 = 0xbeeef78c
     r8 = 0x4af2dce0    r9 = 0x00000001   r10 = 0x4188a874    fp = 0xbeeef7b0
     sp = 0xbeeef708    pc = 0x41068651
    Found by: call frame info
 3  libxul.so!mozilla::dom::devicestorage::PDeviceStorageRequestParent::Write [PDeviceStorageRequestParent.cpp : 721 + 0x7]
     r4 = 0xbeeef78c    r5 = 0x441beb00    r6 = 0x45477ee0    r7 = 0xbeeef78c
     r8 = 0x4af2dce0    r9 = 0x00000001   r10 = 0x4188a874    fp = 0xbeeef7b0
     sp = 0xbeeef728    pc = 0x4106867f
    Found by: call frame info
 4  libxul.so!mozilla::dom::devicestorage::PDeviceStorageRequestParent::Write [PDeviceStorageRequestParent.cpp : 495 + 0x9]
     r4 = 0xbeeef78c    r5 = 0x441beb00    r6 = 0x45477ee0    r7 = 0xbeeef78c
     r8 = 0x4af2dce0    r9 = 0x00000001   r10 = 0x4188a874    fp = 0xbeeef7b0
     sp = 0xbeeef730    pc = 0x41068833
    Found by: call frame info
 5  libxul.so!mozilla::dom::devicestorage::PDeviceStorageRequestParent::Send__delete__ [PDeviceStorageRequestParent.cpp : 82 + 0x9]
     r4 = 0x45477ee0    r5 = 0x441beb00    r6 = 0x00000000    r7 = 0xbeeef78c
     r8 = 0x4af2dce0    r9 = 0x00000001   r10 = 0x4188a874    fp = 0xbeeef7b0
     sp = 0xbeeef750    pc = 0x41068965
    Found by: call frame info

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Looks like this must be happening at [1]

294   ContentParent* cp = static_cast<ContentParent*>(mParent->Manager());
295   BlobParent* actor = cp->GetOrCreateActorForBlob(blob);
296 
297   BlobResponse response;
298   response.blobParent() = actor;
299 
300   unused <<  mParent->Send__delete__(mParent, response);

bent, this is the same kind of code that we "fixed" previously by adding a null check and bailing, right?  Maybe we should switch to an error response if we can't create the blob?

[1] http://mxr.mozilla.org/mozilla-central/source/dom/devicestorage/DeviceStorageRequestParent.cpp#295

Comment 2

[Doug Turner (:dougt)]

ugh... looks like everywhere else, we check for a null actor.

And here we only assert:

http://mxr.mozilla.org/mozilla-central/source/dom/indexedDB/IDBObjectStore.cpp#3009

Comment 3

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 4

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

Comment on attachment 703169 [details] [diff] [review]
patch v.1

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

r=me with this change:

::: dom/indexedDB/IDBObjectStore.cpp
@@ +3008,5 @@
>        BlobChild* actor =
>          contentChild->GetOrCreateActorForBlob(file.mFile);
>        NS_ASSERTION(actor, "This should never fail without aborting!");
> +      if (!actor) {
> +        continue;

This should return an error instead of continuing. Also, can you remove the assertion above?

Comment 5

[Doug Turner (:dougt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/735fee512096
https://hg.mozilla.org/releases/mozilla-b2g18/rev/172e7317bc3e

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/735fee512096

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

Landed on mozilla-b2g18/gaia master prior to the 1/25 branching to mozilla-b2g18_v1_0_0/v1.0.0, updating status-b2g-v1.0.0 to fixed.

Comment 8

[Tracy Walker [:tracy]]

verified with 1.1 build of 20130322070202

assumes stability tests of this have passed since fix was landed.