Closed Bug 997242 Opened 6 years ago Closed 6 years ago

crash in nsRefPtr<DeviceStorageFileDescriptor>::~nsRefPtr | libxul.so@0xa39019 | libxul.so@0xa3903f | AsyncLatencyLogger::Release()

Categories

(Core :: DOM: Device Interfaces, defect, critical)

ARM
Gonk (Firefox OS)
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla31
blocking-b2g 1.4+
Tracking Status
firefox29 --- unaffected
firefox30 --- fixed
firefox31 --- fixed
firefox-esr24 --- unaffected
b2g-v1.2 --- unaffected
b2g-v1.3 --- unaffected
b2g-v1.3T --- unaffected
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: jschmitt, Assigned: dhylands)

References

Details

(6 keywords)

Crash Data

Attachments

(2 files)

This bug was filed from the Socorro interface and is 
report bp-00190563-f06e-4a4b-9035-ffdef2140416.
=============================================================

Description:
Camera app crashes when trying to record a video.

Repro Steps:
1) Update a Buri to BuildID: 20140416040204
2) Open Camera app
3) Tap the video/camera icon to switch to video
4) Select the circle button to record a video

Actual:
Camera app crashes upon recording a video

Expected:
The Camera app does not crash

1.5 Environmental Variables:
Device: Buri 1.5 MOZ
BuildID: 20140416040204
Gaia: 3d4b4b06475d2376bad23ac46da185cd48a68d17
Gecko: dd50745d7f35
Version: 31.0a1
Firmware Version: v1.2-device.cfg

Notes:
Repro frequency: 100%
Issue does not repro on 1.4

1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140416000203
Gaia: 441c4bcd8ac4f8c01a9bc5a2f8d64eaa87844803
Gecko: a559848a8a00
Version: 30.0a2
Firmware Version: V1.2-device.cfg
blocking-b2g: --- → 2.0?
QA Contact: mvaughan
Flags: needinfo?(dhylands)
Duplicate of this bug: 997103
While trying to find the regression window, I've found that this crash will not reproduce using engineering builds and therefore had to use production Mozilla inbound builds. This in turn makes the window and push log a little larger than desired.

NIGHTLY (RIL):
This looks to be a gecko issue.

last working gaia/first broken gecko = REPRO
Gaia  553da99ab09b6b894d9f95bb06b16b6e1ddbf0a1
Gecko dd50745d7f35

first broken gaia/last working gecko = NO REPRO
Gaia  3d4b4b06475d2376bad23ac46da185cd48a68d17
Gecko 5b6e82e7bbbf

MOZILLA INBOUND:
- Last Working -
Device: Buri Master MOZ RIL
BuildID: 20140415120004
Gaia: 3d47c0627017ef77b1adf179792c6543a349af72
Gecko: 3ee388876965
Version: 31.0a1
Firmware Version: v1.2-device.cfg

- First Broken -
Device: Buri Master MOZ RIL
BuildID: 20140415180003
Gaia: 3d47c0627017ef77b1adf179792c6543a349af72
Gecko: 1a2e5e4a6760
Version: 31.0a1
Firmware Version: v1.2-device.cfg

Push log: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3ee388876965&tochange=1a2e5e4a6760
Attaching logcat in case it is useful - sometimes the crash information doesn't have all we need.
I was able to reproduce on my eng build

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 3726.3755]
0x4108e058 in DeviceStorageFileDescriptor::Release (this=0x44c11b78, __in_chrg=<value optimized out>) at /home/work/B2G-hamachi/b2g-inbound/dom/devicestorage/DeviceStorageFileDescriptor.h:14
14	  NS_INLINE_DECL_REFCOUNTING(DeviceStorageFileDescriptor)
(gdb) bt
#0  0x4108e058 in DeviceStorageFileDescriptor::Release (this=0x44c11b78, __in_chrg=<value optimized out>) at /home/work/B2G-hamachi/b2g-inbound/dom/devicestorage/DeviceStorageFileDescriptor.h:14
#1  ~nsRefPtr (this=0x44c11b78, __in_chrg=<value optimized out>) at ../../dist/include/nsAutoPtr.h:894
#2  0x411bb1c2 in ~Message (this=0x44c11b50, __in_chrg=<value optimized out>) at /home/work/B2G-hamachi/b2g-inbound/dom/camera/CameraControlImpl.cpp:512
#3  0x411bb1d8 in ~Message (this=0x424024b5, __in_chrg=<value optimized out>) at /home/work/B2G-hamachi/b2g-inbound/dom/camera/CameraControlImpl.cpp:512
#4  0x406cf4bc in nsRunnable::Release (this=0x44c11b50) at /home/work/B2G-hamachi/b2g-inbound/xpcom/glue/nsThreadUtils.cpp:32
#5  0x406c0eea in ~nsCOMPtr (this=0x46d25dd0, __in_chrg=<value optimized out>) at ../../../dist/include/nsCOMPtr.h:532
#6  0x40720706 in nsThread::ProcessNextEvent (this=0x4510de80, mayWait=true, result=0x46d25e0f) at /home/work/B2G-hamachi/b2g-inbound/xpcom/threads/nsThread.cpp:704
#7  0x406cf79c in NS_ProcessNextEvent (thread=0x4510de80, mayWait=true) at /home/work/B2G-hamachi/b2g-inbound/xpcom/glue/nsThreadUtils.cpp:263
#8  0x408fd55a in mozilla::ipc::MessagePumpForNonMainThreads::Run (this=0x451aff70, aDelegate=0x451737c0) at /home/work/B2G-hamachi/b2g-inbound/ipc/glue/MessagePump.cpp:336
#9  0x408e8a6e in MessageLoop::RunInternal (this=0x451737c0) at /home/work/B2G-hamachi/b2g-inbound/ipc/chromium/src/base/message_loop.cc:226
#10 0x408e8a86 in MessageLoop::RunHandler (this=0x451737c0) at /home/work/B2G-hamachi/b2g-inbound/ipc/chromium/src/base/message_loop.cc:219
#11 MessageLoop::Run (this=0x451737c0) at /home/work/B2G-hamachi/b2g-inbound/ipc/chromium/src/base/message_loop.cc:193
#12 0x4071fb86 in nsThread::ThreadFunc (arg=<value optimized out>) at /home/work/B2G-hamachi/b2g-inbound/xpcom/threads/nsThread.cpp:311
#13 0x42e37744 in _pt_root (arg=<value optimized out>) at /home/work/B2G-hamachi/b2g-inbound/nsprpub/pr/src/pthreads/ptthread.c:212
#14 0x400d6114 in __thread_entry (func=0x42e37699 <_pt_root>, arg=0x451ed780, tls=<value optimized out>) at bionic/libc/bionic/pthread.c:217
#15 0x400d5c68 in pthread_create (thread_out=<value optimized out>, attr=0xbef61b6c, start_routine=0x42e37699 <_pt_root>, arg=0x451ed780) at bionic/libc/bionic/pthread.c:357
#16 0x00000000 in ?? ()
Flags: needinfo?(dhylands)
I found this in the log:

MOZ_CRASH(DeviceStorageFileDescriptor not thread-safe) at ../../dist/include/DeviceStorageFileDescriptor.h:23
https://hg.mozilla.org/mozilla-central/rev/b54fdf8bb486 exposed this today, but the underlying bug (doing reference counting on multiple threads) likely goes back farther.
Component: Gaia::Camera → DOM: Device Interfaces
Product: Firefox OS → Core
Assignee: nobody → dhylands
Making the reference count threadsafe allows a video to be recorded.

As suggested in comment 7, the underlying problem has been present since the code was written (my bad).
(In reply to Dave Hylands [:dhylands] (away - back Tue April 15) from comment #9)
> Making the reference count threadsafe allows a video to be recorded.
> 
> As suggested in comment 7, the underlying problem has been present since the
> code was written (my bad).

It's not your fault, it's the lack of the damn assertions.
Duplicate of this bug: 997910
Comment on attachment 8407862 [details] [diff] [review]
Make DeviceStorageFileDescriptor use threadsafe reference count

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

I wanted to spend more time investigating the threadsafety here, but I am going on vacation for a few weeks at the end of the day and this needs to land now.
Attachment #8407862 - Flags: review?(khuey) → review+
Bug occurs on the Open C as well, adding the signature generated by that crash.
Crash Signature: [@ nsRefPtr<DeviceStorageFileDescriptor>::~nsRefPtr | libxul.so@0xa39019 | libxul.so@0xa3903f | AsyncLatencyLogger::Release()] → [@ nsRefPtr<DeviceStorageFileDescriptor>::~nsRefPtr | libxul.so@0xa39019 | libxul.so@0xa3903f | AsyncLatencyLogger::Release()] [@ DeviceStorageFileDescriptor::Release()]
https://hg.mozilla.org/mozilla-central/rev/1f2b13a29a74

This affects v1.4 as well, right?
Status: NEW → RESOLVED
Closed: 6 years ago
status-b2g-v1.4: --- → ?
Flags: needinfo?(dhylands)
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Bug 991812 is the bug that introduced the original problem.

It doesn't look like bug 991812 landed on 1.4 (but I could easily be misreading something).
Flags: needinfo?(dhylands)
No, bug 991812 introduces assertions that catch the problem.  I don't know when the thread-safety bug was introduced.
I'm going to guess that this goes back to the initial landing of DeviceStorageFileDescriptor in bug 910498.  I can't imagine the threadedness of it has changed, but please correct me if I'm wrong.  It looks like that bug landed in 1.3.
Yes - Andrew is right. To be safe, it would be best to backport it. I wasn't thinking about this correctly when I made comment 17.
Bug 910498 didn't land on 1.3 (at least when I checked there was no dom/devicestorage/DeviceStorageFileDescriptor.h file).
https://git.mozilla.org/?p=releases/gecko.git;a=tree;f=dom/devicestorage;h=9627d2c7a4899c59718dc8d0ee84ab02a3596e2e;hb=v1.3

So marking 1.3 and 1.3T as unaffected
I confirmed that the patch for master builds on a 1.4 branch
Should we switch this to a 1.4 blocker given that this technically affects 1.4?
blocking-b2g: 2.0? → 2.0+
Tested and working
1.5
Hamachi
Gecko 22b3e73
Gaia f046370
blocking-b2g: 1.4? → 1.4+
Tested and working
1.5
Hamachi
Gecko: 7db76c3
Gaia: facd91d

1.4
Hamachi
Gecko: 1ed84a6
Gaia: 4eb2393
Status: RESOLVED → VERIFIED
Group: core-security
You need to log in before you can comment on or make changes to this bug.