Closed Bug 1003658 Opened 6 years ago Closed 6 years ago

[Bluedroid] The memory of BT profile controller and ReplyRunnable for ConnectRequest wouldn't be released and becomes a leak.

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

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

RESOLVED FIXED
2.0 S1 (9may)
blocking-b2g 1.4+
Tracking Status
firefox30 --- wontfix
firefox31 --- wontfix
firefox32 --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: jaliu, Assigned: jaliu)

References

Details

(Whiteboard: [p=1], [ETA:5/8])

Attachments

(1 file, 2 obsolete files)

The memory of BluetoothRequestParent::ReplyRunnable for ConnectRequest wouldn't be released and becomes a leak. Please refer to ipc/BluetoothParent.cpp

STR:
1. Enable BT
2. Pair and connect to a profile. e.g HFP profile, A2DP profile
2. Disable BT
3. Kill setting app
4. Check the log in destructor of BluetoothRequestParent::ReplyRunnable
Assignee: nobody → jaliu
Summary: The memory of ReplyRunnable for ConnectRequest wouldn't be released and becomes a leak. → [Bluedroid] The memory of ReplyRunnable for ConnectRequest wouldn't be released and becomes a leak.
Status: NEW → ASSIGNED
Attachment #8415026 - Flags: review?(echou)
Attachment #8415026 - Flags: feedback?(shuang)
See Also: → 997962
blocking-b2g: --- → 1.4+
blocking-b2g: 1.4+ → 1.4?
blocking-b2g: 1.4? → 1.4+
Comment on attachment 8415026 [details] [diff] [review]
Correct reference count to fix the memory leak of BT profile controller. (v0)

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

Hi Jamin,

r- because I don't think there is a memory leak with current implementation. I might be wrong, but would you mind taking a look again?

::: dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp
@@ +1157,1 @@
>    sControllerArray.RemoveElementAt(0);

Function RemoveElementAt() would call function DestructRange(start, count), DestructRange() calls elem_traits::Destruct(), which is ~nsRefPtr() in this case. The dtor of nsRefPtr looks like:

  ~nsRefPtr()
  {
    if ( mRawPtr )
      mRawPtr->Release();
  }

It seems that the reference count of the object, which is the first(0) element of sControllerArray, would be decreased by 1, just does the same thing as null it out.
Attachment #8415026 - Flags: review?(echou) → review-
Whiteboard: [p=1]
Whiteboard: [p=1] → [p=1], [ETA:5/8]
Summary: [Bluedroid] The memory of ReplyRunnable for ConnectRequest wouldn't be released and becomes a leak. → [Bluedroid] The memory of BT profile controller and ReplyRunnable for ConnectRequest wouldn't be released and becomes a leak.
Update the summary of bug since the BT profile controller would become a leak too.
Attachment #8415026 - Attachment is obsolete: true
Attachment #8417934 - Flags: review?(echou)
Attachment #8417934 - Flags: feedback?(shuang)
The root cause of this bug is that BluetoothProfileController is in a reference cycle.
BluetoothProfileController holds the reference of CheckProfileStatusCallback and CheckProfileStatusCallback also holds the reference of BluetoothProfileController. Since the reference count can't decrease to zero, they would become a memory leaks.

The reference of BluetoothReplyRunnable would be held by BluetoothProfileController too, thus, it can't be released too.
Comment on attachment 8417934 [details] [diff] [review]
Avoid to create reference cycle in BluetoothProfileController which would cause memory leak. (v1)

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

Good catch. :)
Attachment #8417934 - Flags: review?(echou) → review+
Keywords: checkin-needed
Target Milestone: --- → 2.0 S1 (9may)
https://hg.mozilla.org/mozilla-central/rev/8cf161cd53a3
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.