Closed
Bug 949772
Opened 10 years ago
Closed 10 years ago
MOZ_ASSERT(arr.Length() == 1) in mozilla::dom::bluetooth::BluetoothAdapter::Notify
Categories
(Firefox OS Graveyard :: Bluetooth, defect)
Tracking
(blocking-b2g:-)
RESOLVED
FIXED
blocking-b2g | - |
People
(Reporter: gwagner, Assigned: echou)
Details
Attachments
(2 files, 3 obsolete files)
3.47 KB,
patch
|
Details | Diff | Splinter Review | |
6.64 KB,
patch
|
Details | Diff | Splinter Review |
Seen on nexus4 with current gecko/gaia trunk and debug gecko build. STR: start music app Enable/disable nfc, bluetooth a few times: Program received signal SIGSEGV, Segmentation fault. 0xb554985a in mozilla::dom::bluetooth::BluetoothAdapter::Notify (this=0xb1fd3c00, aData=...) at ../../../dom/bluetooth/BluetoothAdapter.cpp:321 321 MOZ_ASSERT(arr.Length() == 1); (gdb) bt #0 0xb554985a in mozilla::dom::bluetooth::BluetoothAdapter::Notify (this=0xb1fd3c00, aData=...) at ../../../dom/bluetooth/BluetoothAdapter.cpp:321 #1 0xb554dedc in Broadcast (aParam=<optimized out>, this=<optimized out>) at ../../dist/include/mozilla/Observer.h:67 #2 mozilla::dom::bluetooth::BluetoothService::DistributeSignal (this=<optimized out>, aSignal=...) at ../../../dom/bluetooth/BluetoothService.cpp:458 #3 0xb555051a in mozilla::dom::bluetooth::BluetoothChild::RecvNotify (this=<optimized out>, aSignal=<optimized out>) at ../../../dom/bluetooth/ipc/BluetoothChild.cpp:78 #4 0xb4fb84d2 in mozilla::dom::bluetooth::PBluetoothChild::OnMessageReceived (this=0xb1f43af0, __msg=...) at PBluetoothChild.cpp:364 #5 0xb4fdd6ae in mozilla::dom::PContentChild::OnMessageReceived (this=0xb3e44818, __msg=...) at PContentChild.cpp:3155 #6 0xb4f9405c in mozilla::ipc::MessageChannel::DispatchAsyncMessage (this=0xb3e44848, aMsg=...) at ../../../ipc/glue/MessageChannel.cpp:986 #7 0xb4f96834 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne (this=0xb3e44848) at ../../../ipc/glue/MessageChannel.cpp:887 #8 0xb4f93954 in DispatchToMethod<mozilla::ipc::MessageChannel, void (mozilla::ipc::MessageChannel::*)()> (method= (void (mozilla::ipc::MessageChannel::*)(mozilla::ipc::MessageChannel * const)) 0xb4f9679d <mozilla::ipc::MessageChannel::OnMaybeDequeueOne()>, obj=<optimized out>, arg=<optimized out>) at ../../../ipc/chromium/src/base/tuple.h:383 #9 RunnableMethod<mozilla::ipc::MessageChannel, void (mozilla::ipc::MessageChannel::*)(), Tuple0>::Run (this=<optimized out>) at ../../../ipc/chromium/src/base/task.h:307 #10 0xb4f944c0 in Run (this=<optimized out>) at ../../dist/include/mozilla/ipc/MessageChannel.h:441 #11 mozilla::ipc::MessageChannel::DequeueTask::Run (this=<optimized out>) at ../../dist/include/mozilla/ipc/MessageChannel.h:458 #12 0xb4f8a318 in MessageLoop::RunTask (this=0xbe95af20, task=0xb2058900) at ../../../ipc/chromium/src/base/message_loop.cc:344 #13 0xb4f8ac56 in MessageLoop::DeferOrRunPendingTask (this=<optimized out>, pending_task=<optimized out>) at ../../../ipc/chromium/src/base/message_loop.cc:352 #14 0xb4f8bcfa in DoWork (this=<optimized out>) at ../../../ipc/chromium/src/base/message_loop.cc:452 #15 MessageLoop::DoWork (this=0xbe95af20) at ../../../ipc/chromium/src/base/message_loop.cc:431 #16 0xb4f9788e in mozilla::ipc::DoWorkRunnable::Run (this=<optimized out>) at ../../../ipc/glue/MessagePump.cpp:226 #17 0xb4e0cadc in ProcessNextEvent (result=0xbe95adc7, mayWait=<optimized out>, this=0xb3e2e800) at ../../../xpcom/threads/nsThread.cpp:634 #18 nsThread::ProcessNextEvent (this=0xb3e2e800, mayWait=<optimized out>, result=0xbe95adc7) at ../../../xpcom/threads/nsThread.cpp:567 #19 0xb4dcb0cc in NS_ProcessNextEvent (thread=0xb3e2e800, mayWait=<optimized out>) at ../../../xpcom/glue/nsThreadUtils.cpp:263
Assignee: nobody → shuang
blocking-b2g: --- → 1.3?
blocking-b2g: 1.3? → ---
Thanks for reporting. I will check later.
Assignee: shuang → nobody
Assignee | ||
Comment 3•10 years ago
|
||
I was unable to reproduce, but I did find something wrong and I believe that would be the root cause which make this occur. The patch is coming.
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8347952 -
Flags: review?(gyeh)
Attachment #8347952 -
Flags: feedback?(btian)
Comment 5•10 years ago
|
||
Comment on attachment 8347952 [details] [diff] [review] patch 1: v1: event PropertyChanged should only contains one element Review of attachment 8347952 [details] [diff] [review]: ----------------------------------------------------------------- LGTM.
Attachment #8347952 -
Flags: feedback?(btian) → feedback+
Comment 6•10 years ago
|
||
Comment on attachment 8347952 [details] [diff] [review] patch 1: v1: event PropertyChanged should only contains one element Review of attachment 8347952 [details] [diff] [review]: ----------------------------------------------------------------- How about handling multiple |PropertyChanged| sequentially in BluetoothAdapter? When backend returns several pairs of name and value at one time, we just need to broadcast the signal once and let each instance of BluetoothAdapter to update its own values.
Assignee | ||
Comment 7•10 years ago
|
||
* Updated based on Gina's comment.
Attachment #8347952 -
Attachment is obsolete: true
Attachment #8347952 -
Flags: review?(gyeh)
Attachment #8347967 -
Flags: review?(gyeh)
Attachment #8347967 -
Flags: feedback?(btian)
Comment 8•10 years ago
|
||
Comment on attachment 8347967 [details] [diff] [review] patch 1: v2: Make BluetoothAdpater be able to deal with multiple properties in one PropertyChanged event Review of attachment 8347967 [details] [diff] [review]: ----------------------------------------------------------------- Nothing to pick on. :)
Attachment #8347967 -
Flags: review?(gyeh) → review+
Comment 9•10 years ago
|
||
Comment on attachment 8347967 [details] [diff] [review] patch 1: v2: Make BluetoothAdpater be able to deal with multiple properties in one PropertyChanged event Review of attachment 8347967 [details] [diff] [review]: ----------------------------------------------------------------- f=me with nit addressed. ::: dom/bluetooth/BluetoothAdapter.cpp @@ +321,2 @@ > > + for (uint32_t i = 0; i < propertyCount; ++i) { You can check i < arr.length() directly.
Attachment #8347967 -
Flags: feedback?(btian) → feedback+
Assignee | ||
Comment 10•10 years ago
|
||
Nominate as 1.3+ since 1.3 would be affected.
blocking-b2g: --- → 1.3?
Assignee | ||
Comment 11•10 years ago
|
||
> ::: dom/bluetooth/BluetoothAdapter.cpp
> @@ +321,2 @@
> >
> > + for (uint32_t i = 0; i < propertyCount; ++i) {
>
> You can check i < arr.length() directly.
Creating a variable must be more efficient(or at least won't be less efficient) than getting unchanged length by calling length() every time.
Assignee | ||
Comment 13•10 years ago
|
||
* Per offline discussion with Gina, we should notify registering BluetoothDevice objects as well as what we do to BluetoothAdapter.
Attachment #8348549 -
Flags: review?(gyeh)
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Eric Chou [:ericchou] [:echou] from comment #13) > Created attachment 8348549 [details] [diff] [review] > patch 2: v1: Fire PropertyChanged to registering BluetoothDevice objects > > * Per offline discussion with Gina, we should notify registering > BluetoothDevice objects as well as what we do to BluetoothAdapter. In addition, 2 callback functions are renamed in this patch since they are just callbacks to setter and getter, 'Changed' shouldn't be necessary.
Comment 15•10 years ago
|
||
triage: not blocking release as there seem to be no major user impact. please work on trunk and ask for approval to land in 1.3
blocking-b2g: 1.3? → -
Comment 16•10 years ago
|
||
Comment on attachment 8348549 [details] [diff] [review] patch 2: v1: Fire PropertyChanged to registering BluetoothDevice objects Review of attachment 8348549 [details] [diff] [review]: ----------------------------------------------------------------- r+ with nits addressed. ::: dom/bluetooth/bluedroid/gonk/BluetoothServiceBluedroid.cpp @@ +385,1 @@ > * following conditions: nit: Please merge them into one line. @@ +427,4 @@ > } > } > > + // Update to registering BluetoothDevice objects nit: registered @@ +429,5 @@ > > + // Update to registering BluetoothDevice objects > + BluetoothValue value(props); > + BluetoothSignal signal(NS_LITERAL_STRING("PropertyChanged"), > + remoteDeviceBdAddress, value); nit: You can use anonymous object for |value| if you don't need it after creating BluetoothSignal.
Attachment #8348549 -
Flags: review?(gyeh) → review+
Assignee | ||
Comment 17•10 years ago
|
||
* nits picked.
Attachment #8348549 -
Attachment is obsolete: true
Assignee | ||
Comment 18•10 years ago
|
||
try: https://tbpl.mozilla.org/?tree=Try&rev=f71df01e5829
Assignee | ||
Comment 19•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/f04d4d42ac8a https://hg.mozilla.org/integration/b2g-inbound/rev/32a40f2c842c
Comment 20•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f04d4d42ac8a https://hg.mozilla.org/mozilla-central/rev/32a40f2c842c
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•