Closed Bug 827888 Opened 7 years ago Closed 7 years ago

Race condition in Bluetooth DBUS connection

Categories

(Firefox OS Graveyard :: General, defect, major)

ARM
Gonk (Firefox OS)
defect
Not set
major

Tracking

(firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g1819+ fixed, b2g18-v1.0.0 wontfix)

RESOLVED FIXED
B2G C4 (2jan on)
Tracking Status
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- fixed
b2g18 19+ fixed
b2g18-v1.0.0 --- wontfix

People

(Reporter: tzimmermann, Assigned: tzimmermann)

Details

Attachments

(2 files, 2 obsolete files)

When pairing with other Bluetooth devices, it can happen that the pair response receives before the message handler is set up. In this case the message is lost and the device thinks it's still pairing.

We need a reliable way of setting up the message handler. DBUS doesn't provide this feature for us.
You can trigger this bug when quickly trying to connect to the same peer multiple times; as described in bug 826122.
Assignee: echou → tzimmermann
This is a workaround for the problem that increases success rate from ~10% to ~75%.
Attachment #699761 - Flags: review?(echou)
Comment on attachment 699761 [details] [diff] [review]
Explicitly call notifier if DBUS call completes early

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

Looks good to me, but I think it would be better if Kyle can take a look at this since he is the original author. :)
Attachment #699761 - Flags: review?(kyle)
Attachment #699761 - Flags: review?(echou)
Attachment #699761 - Flags: review+
Attachment #699761 - Flags: review?(kyle) → review+
Attached patch Fix usage of sIsParing variable (obsolete) — Splinter Review
This patch fixes another race condition during the pairing setup and allows to run several pairing requests in parallel. With both patches applied, I wasn't able to reproduce the original problem any longer.
Attachment #699827 - Flags: review?(kyle)
Attachment #699827 - Flags: review?(echou)
Attachment #699827 - Flags: review?(echou) → review+
Attachment #699827 - Flags: review?(kyle) → review+
Thanks everyone for your reviews.
Keywords: checkin-needed
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Bluetooth pairing might fail with no or incorrect error
Testing completed: On my unagi
Risk to taking this patch (and alternatives if risky): Low
String or UUID changes made by this patch: -
Attachment #699761 - Attachment is obsolete: true
Attachment #700308 - Flags: review+
Attachment #700308 - Flags: approval-mozilla-b2g18?
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Bluetooth pairing might fail with no or incorrect error; multiple concurrent pairing requests will interfere
Testing completed: On my unagi
Risk to taking this patch (and alternatives if risky): Low
String or UUID changes made by this patch: -
Attachment #699827 - Attachment is obsolete: true
Attachment #700310 - Flags: review+
Attachment #700310 - Flags: approval-mozilla-b2g18?
(once this gets approval, please add checkin-needed again for the b2g18 landing)
For really fixing this problem, we'd need to handle all interaction with DBus from within a single thread. I currently work on converting the DBus event loop into an nsRunnable and scheduling DBus requests within the same thread context. Patches will follow soon. I hope to have this ready for v2.
Leaving this nom for approval, given that it's not blocking and is a fairly large change in the code we'll wait until 1.0.1 for this.  See https://wiki.mozilla.org/Release_Management/B2G_Landing for more details but basically the options are to wait until b2g18 opens up for 1.0.1 landings after 1/25 or land now to the date project branch so it gets merged in after 1/25.
Attachment #700308 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
Comment on attachment 700310 [details] [diff] [review]
Fix usage of sIsParing variable

Please go ahead with uplift to the tip of mozilla-b2g18 branch for v1.0.1
Attachment #700310 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
You need to log in before you can comment on or make changes to this bug.