Closed Bug 985949 Opened 6 years ago Closed 6 years ago

[Bluetooth] BlueZ backend crashes during pointer handling

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.4+, firefox29 wontfix, firefox30 verified, firefox31 verified, b2g-v1.4 fixed, b2g-v2.0 fixed)

VERIFIED FIXED
1.4 S4 (28mar)
blocking-b2g 1.4+
Tracking Status
firefox29 --- wontfix
firefox30 --- verified
firefox31 --- verified
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: tzimmermann, Assigned: tzimmermann)

References

Details

(Keywords: crash, regression)

Attachments

(2 files, 1 obsolete file)

This is fallout from bug 967364. When handing over pointers to DBus operations, BlueZ crashes during pointer cleanups. The stack is

(gdb) c
Continuing.
[New Thread 781.832]
[New Thread 781.796]

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 781.796]
0x415bbf04 in ~already_AddRefed (this=0x44cc2e00) at ../../dist/include/nsCOMPtr.h:177
177	      MOZ_ASSERT(!mRawPtr);
(gdb) bt
#0  0x415bbf04 in ~already_AddRefed (this=0x44cc2e00) at ../../dist/include/nsCOMPtr.h:177
#1  AddReservedServiceRecordsTask::Run (this=0x44cc2e00) at /home/mozilla/Projects/mozilla/src/mozilla-central/dom/bluetooth/bluez/BluetoothDBusService.cpp:1532
#2  0x40e326f4 in MessageLoop::RunTask (this=0x43d78dc4, task=0x44cc2e00) at /home/mozilla/Projects/mozilla/src/mozilla-central/ipc/chromium/src/base/message_loop.cc:344
#3  0x40e32dfe in MessageLoop::DeferOrRunPendingTask (this=0x4f, pending_task=<value optimized out>)
    at /home/mozilla/Projects/mozilla/src/mozilla-central/ipc/chromium/src/base/message_loop.cc:352
#4  0x40e33a44 in MessageLoop::DoWork (this=0x43d78dc4) at /home/mozilla/Projects/mozilla/src/mozilla-central/ipc/chromium/src/base/message_loop.cc:430
#5  0x40e28b16 in base::MessagePumpLibevent::Run (this=0x40301e20, delegate=0x43d78dc4)
    at /home/mozilla/Projects/mozilla/src/mozilla-central/ipc/chromium/src/base/message_pump_libevent.cc:311
#6  0x40e32c06 in MessageLoop::RunInternal (this=0x43d78dc4) at /home/mozilla/Projects/mozilla/src/mozilla-central/ipc/chromium/src/base/message_loop.cc:226
#7  0x40e32c1e in MessageLoop::RunHandler (this=0x43d78dc4) at /home/mozilla/Projects/mozilla/src/mozilla-central/ipc/chromium/src/base/message_loop.cc:219
#8  MessageLoop::Run (this=0x43d78dc4) at /home/mozilla/Projects/mozilla/src/mozilla-central/ipc/chromium/src/base/message_loop.cc:193
#9  0x40e35a3a in base::Thread::ThreadMain (this=0x4031c380) at /home/mozilla/Projects/mozilla/src/mozilla-central/ipc/chromium/src/base/thread.cc:162
#10 0x40e2912c in ThreadFunc (closure=0x1) at /home/mozilla/Projects/mozilla/src/mozilla-central/ipc/chromium/src/base/platform_thread_posix.cc:39
#11 0x40120e18 in __thread_entry (func=0x40e29125 <ThreadFunc>, arg=0x4031c380, tls=<value optimized out>) at bionic/libc/bionic/pthread.c:217
#12 0x4012096c in pthread_create (thread_out=<value optimized out>, attr=0xbe9677f8, start_routine=0x40e29125 <ThreadFunc>, arg=0x4031c380)
    at bionic/libc/bionic/pthread.c:357
#13 0x00000000 in ?? ()


STR:

  - build BlueZ with bug 967364 applied
  - turn on Bluetooth

Expected:

  - it works

Actual result:

  - BT crashes immediately
Kyle, we need to clear nsRefPtrs here without decrementing the reference counter. The patch swaps them with a cleared pointer, which work. Is that OK, or is there a better pattern?
Attachment #8394151 - Flags: review?(echou)
Attachment #8394151 - Flags: feedback?(khuey)
Comment on attachment 8394151 [details] [diff] [review]
[01] Bug 985949: Fix pointer handling in BlueZ backend

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

You can change

refPtr.forget();

to

unused << refPtr.forget();

And add comments about where the Release() happens.

(Might need to include mozilla/unused.h and grab unused from the mozilla namespace)
Attachment #8394151 - Flags: feedback?(khuey) → feedback-
Comment on attachment 8394151 [details] [diff] [review]
[01] Bug 985949: Fix pointer handling in BlueZ backend

OK, thanks. Removing r? for now.
Attachment #8394151 - Flags: review?(echou)
Changes since v1:

  - shift |unused| by value, as outlined in the feedback
Attachment #8394151 - Attachment is obsolete: true
Attachment #8394194 - Flags: review?(echou)
Comment on attachment 8394194 [details] [diff] [review]
[01] Bug 985949: Fix pointer handling in BlueZ backend (v2)

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

Thanks!
Attachment #8394194 - Flags: review?(echou) → review+
Attachment #8395535 - Flags: review?(echou) → review+
Duplicate of this bug: 984733
Migrating flags over.
blocking-b2g: --- → 1.4+
Keywords: crash, regression
Duplicate of this bug: 983590
Keywords: verifyme
Verified fixed.

I've tested on Open_C device (JB device, uses bluez as back-end according to https://bugzilla.mozilla.org/show_bug.cgi?id=984733#c3)

STR:
1. on Ubuntu --> enable BT
2. on Open_C --> Settings --> Enable BT --> send pairing request to Ubuntu BT
==> pairs successfully without crashes

Open_C 1.4
BuildID: 20140506000202
Gaia: b1242f33981024de59b8b4c26bacff8b876211b1
Gecko: fe4080728c60
Version: 30.0

Open_C master m-c
BuildID: 20140506040204
Gaia: 4af716f09747edfbea637f5b60f7fd7d0183f19b
Gecko: 81651ad5e43c
Version: 32.0a1

Base: P821A10V1.0.0B06_LOG_DL
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.