[Bluetooth] MOZ_ASSERT fails in BluetoothOppManager::Disconnect() on debug build

RESOLVED FIXED

Status

Firefox OS
Bluetooth
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: btian, Assigned: btian)

Tracking

({regression})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

4 years ago
This bug is regression of bug 897782. Disconnect() is still called internally in BluetoothOppManager. Need to implement socket disconnection for it.
(Assignee)

Comment 1

4 years ago
Created attachment 830772 [details] [diff] [review]
Patch 1 (v1): socket disconnection in OPP Disconnect()
Assignee: nobody → btian
Keywords: regression
(Assignee)

Comment 2

4 years ago
Created attachment 831215 [details] [diff] [review]
Patch 1 (v2): socket disconnection in OPP Disconnect()

Replace Disconnect() with socket disconnection directly.
Attachment #830772 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #831215 - Flags: review?(echou)
Comment on attachment 831215 [details] [diff] [review]
Patch 1 (v2): socket disconnection in OPP Disconnect()

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

r=me with the only problem solved.

::: dom/bluetooth/BluetoothOppManager.cpp
@@ +462,5 @@
>  
>    if (mIsServer) {
>      mAbortFlag = true;
>    } else {
> +    MOZ_ASSERT(mSocket);

Because BluetoothOppManager::StopSendingFile() may be called accidentally from Gaia even when mSocket is invalid, it would be better to block this case instead of only assertion.

if (mIsServer) {
  mAbortFlag = true;
} else if (mSocket) {
  mSocket->Disconnect();
} else {
  // Do nothing(warning, maybe?)
}
Attachment #831215 - Flags: review?(echou) → review+
(Assignee)

Comment 4

4 years ago
Created attachment 832688 [details] [diff] [review]
[final] Patch 1: socket disconnection in OPP Disconnect(), r=echou

Update based on review comment.

try server: https://tbpl.mozilla.org/?tree=Try&rev=54ea0c58f16c
Attachment #831215 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/b2g-inbound/rev/a28dff85006e
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a28dff85006e
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.