Closed Bug 768685 Opened 12 years ago Closed 12 years ago

[b2g-bluetooth] Fix dbus poll thread launching issues

Categories

(Core :: DOM: Device Interfaces, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: qdot, Assigned: qdot)

References

Details

Carryover from bug 740744, Comment #38:

In DBusThread::StartEventLoop you've got a lock that wraps the whole function. In there you're calling socketpair and a series of dbus calls followed by NS_NewThread. Our general rule is to never call system functions with locks held, and in Gecko we try to never call into other modules (maybe loosely defined as any code that is in a different directory these days) with locks held. Basically we want mutexes to be held as briefly as possible to ensure that they can't block for more than a tiny bit.

In the particular case of StartEventLoop there's no real need for the mutex to be held at all... You haven't created the thread that could be racing with you yet, so your lock isn't really doing much for you.

StopEventLoop also locks and then calls write and then spins an event loop while waiting for the thread to shut down which must be fixed.

But I wonder if you really need the mutex at all. You do some up-front work to make the fds that you need, but after that couldn't everything else be handled on the dbus poll thread? If all your mutable state is handled there then you shouldn't need the mutex. You're calling seemingly matched dbus functions on two different threads (e.g. dbus_bus_add_match on parent thread, dbus_bus_remove_match on dbus thread) which may technically be ok but seems mighty confusing. Moving all the mutable stuff to the dbus poll thread would clear that up too.

So for now we have to fix the system-calls-under-mutex, and the event-loop-under-mutex, and the race I found below (mIsRunning). If you want to wait to do the mutex removal/data separation then we should file a followup to give DBusThread a thorough scrubbing.
This was fixed and landed as part of bug 740744. Mutex was simply removed.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.