1233060 - No unprotected references in Bluetooth backend code

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Thomas Zimmermann [:tzimmermann] [:tdz]]

We have unprotected references to result handlers in the Bluetooth backend code. This leads to ref-counting problems like bug 1232670. The correct fix is to always keep classes protected by smart pointers.

Comment 1

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: 01_convert_bluedroid_service.patch

Nathan,

I want to construct objects with |MakeAndAddRef| and pass the raw pointer to a function. But the |already_AddRefed| that is returned by |MakeAndAddRef| cannot be converted to a raw pointer.

So I added a new helper named |MakeRef| that returns a |RefPtr|. I'd rather prefer to change the return type of |MakeAndAddRef| to |RefPtr| and fix the rest of tree where necessary.

I'd like to ask for your opinion on this. Are there better options?

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 8698994 [details] [diff] [review]
01_convert_bluedroid_service.patch

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

I don't think this patch is going in the right direction because we have things like:

http://mxr.mozilla.org/mozilla-central/source/mfbt/RefPtr.h#259

which will make all of the examples herein compile-time errors with new-enough compilers (if they're not already; I don't know what GCC version we're using across our B2G compilers).

I don't think converting MakeAndAddRef to MakeRef is a good idea because then things like:

NS_DispatchToCurrentThread(MakeAndAddRef<T>(...));

and associated event dispatching variants won't work correctly.  And:

NS_DispatchToCurrentThread(MakeRef<T>(...).forget());

is unwieldy.

You could make NS_DispatchToCurrentThread take RefPtr, but that doesn't communicate the transfer of ownership that the current signature does.