Bug 932728: Open only one connection to DBus, r=echou,qdot

Bluetooth maintains two connections to the DBus server and the DBus
system itself maintains a third one. This implies some overhead and
makes the code more difficult to understand.

This patch changes the Bluetooth code to use the connection that is
established by the DBus system.
This commit is contained in:
Thomas Zimmermann 2013-12-12 12:56:00 +01:00
parent 880a39208d
commit d6f5d45a38
5 changed files with 135 additions and 94 deletions

View File

@ -167,12 +167,6 @@ static const char* sBluetoothDBusSignals[] =
"type='signal',interface='org.bluez.Control'"
};
/**
* DBus Connection held for the BluetoothCommandThread to use. Should never be
* used by any other thread.
*/
static nsRefPtr<RawDBusConnection> gThreadConnection;
// Only A2DP and HID are authorized.
static nsTArray<BluetoothServiceClass> sAuthorizedServiceClass;
@ -974,9 +968,9 @@ AppendDeviceName(BluetoothSignal& aSignal)
nsString devicePath = arr[0].value().get_nsString();
nsRefPtr<RawDBusConnection> threadConnection = gThreadConnection;
RawDBusConnection* connection = GetDBusConnection();
if (!threadConnection.get()) {
if (!connection) {
BT_WARNING("%s: DBus connection has been closed.", __FUNCTION__);
return;
}
@ -985,7 +979,7 @@ AppendDeviceName(BluetoothSignal& aSignal)
new AppendDeviceNameReplyHandler(nsCString(DBUS_DEVICE_IFACE),
devicePath, aSignal);
bool success = threadConnection->SendWithReply(
bool success = connection->SendWithReply(
AppendDeviceNameReplyHandler::Callback, handler.get(), 1000,
NS_ConvertUTF16toUTF8(devicePath).get(), DBUS_DEVICE_IFACE,
"GetProperties", DBUS_TYPE_INVALID);
@ -1219,9 +1213,9 @@ public:
return;
}
nsRefPtr<RawDBusConnection> threadConnection = gThreadConnection;
RawDBusConnection* connection = GetDBusConnection();
if (!threadConnection.get()) {
if (!connection) {
BT_WARNING("%s: DBus connection has been closed.", __FUNCTION__);
return;
}
@ -1229,7 +1223,7 @@ public:
// There is no "RegisterAgent" function defined in device interface.
// When we call "CreatePairedDevice", it will do device agent registration
// for us. (See maemo.org/api_refs/5.0/beta/bluez/adapter.html)
if (!dbus_connection_register_object_path(threadConnection->GetConnection(),
if (!dbus_connection_register_object_path(connection->GetConnection(),
KEY_REMOTE_AGENT,
mAgentVTable,
nullptr)) {
@ -1305,9 +1299,9 @@ private:
const char* agentPath = KEY_LOCAL_AGENT;
const char* capabilities = B2G_AGENT_CAPABILITIES;
nsRefPtr<RawDBusConnection> threadConnection = gThreadConnection;
RawDBusConnection* connection = GetDBusConnection();
if (!threadConnection.get()) {
if (!connection) {
BT_WARNING("%s: DBus connection has been closed.", __FUNCTION__);
return false;
}
@ -1318,7 +1312,7 @@ private:
// signal will be passed to local agent. If we start pairing process with
// calling CreatePairedDevice, we'll get signal which should be passed to
// device agent.
if (!dbus_connection_register_object_path(threadConnection->GetConnection(),
if (!dbus_connection_register_object_path(connection->GetConnection(),
KEY_LOCAL_AGENT,
aAgentVTable,
nullptr)) {
@ -1332,7 +1326,7 @@ private:
MOZ_ASSERT(handler.get());
MOZ_ASSERT(!sAdapterPath.IsEmpty());
bool success = threadConnection->SendWithReply(
bool success = connection->SendWithReply(
RegisterAgentReplyHandler::Callback, handler.get(), -1,
NS_ConvertUTF16toUTF8(sAdapterPath).get(),
DBUS_ADAPTER_IFACE, "RegisterAgent",
@ -1367,9 +1361,9 @@ public:
MOZ_ASSERT(NS_IsMainThread());
nsRefPtr<RawDBusConnection> threadConnection = gThreadConnection;
RawDBusConnection* connection = GetDBusConnection();
if (!threadConnection.get()) {
if (!connection) {
BT_WARNING("%s: DBus connection has been closed.", __FUNCTION__);
return NS_ERROR_FAILURE;
}
@ -1383,7 +1377,7 @@ public:
const dbus_uint32_t* services = sServices;
bool success = threadConnection->SendWithReply(
bool success = connection->SendWithReply(
DBusReplyHandler::Callback, handler.get(), -1,
NS_ConvertUTF16toUTF8(sAdapterPath).get(),
DBUS_ADAPTER_IFACE, "AddReservedServiceRecords",
@ -1727,7 +1721,7 @@ OnDefaultAdapterReply(DBusMessage* aReply, void* aData)
bool
BluetoothDBusService::IsReady()
{
if (!IsEnabled() || !mConnection || !gThreadConnection || IsToggling()) {
if (!IsEnabled() || !GetDBusConnection() || IsToggling()) {
BT_WARNING("Bluetooth service is not ready yet!");
return false;
}
@ -1745,25 +1739,8 @@ BluetoothDBusService::StartInternal()
return NS_ERROR_FAILURE;
}
if (mConnection) {
return NS_OK;
}
mConnection = new RawDBusConnection();
if (NS_FAILED(mConnection->EstablishDBusConnection())) {
BT_WARNING("Cannot start Main Thread DBus connection!");
StopDBus();
return NS_ERROR_FAILURE;
}
gThreadConnection = new RawDBusConnection();
if (NS_FAILED(gThreadConnection->EstablishDBusConnection())) {
BT_WARNING("Cannot start Sync Thread DBus connection!");
StopDBus();
return NS_ERROR_FAILURE;
}
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
DBusError err;
dbus_error_init(&err);
@ -1773,7 +1750,7 @@ BluetoothDBusService::StartInternal()
// signals we want, register all of them in this thread at startup.
// The event handler will sort the destinations out as needed.
for (uint32_t i = 0; i < ArrayLength(sBluetoothDBusSignals); ++i) {
dbus_bus_add_match(mConnection->GetConnection(),
dbus_bus_add_match(connection->GetConnection(),
sBluetoothDBusSignals[i],
&err);
if (dbus_error_is_set(&err)) {
@ -1782,7 +1759,7 @@ BluetoothDBusService::StartInternal()
}
// Add a filter for all incoming messages_base
if (!dbus_connection_add_filter(mConnection->GetConnection(),
if (!dbus_connection_add_filter(connection->GetConnection(),
EventFilter, nullptr, nullptr)) {
BT_WARNING("Cannot create DBus Event Filter for DBus Thread!");
return NS_ERROR_FAILURE;
@ -1800,11 +1777,11 @@ BluetoothDBusService::StartInternal()
* explicitly here.
*/
if (sAdapterPath.IsEmpty()) {
bool success = mConnection->SendWithReply(OnDefaultAdapterReply, nullptr,
1000, "/",
DBUS_MANAGER_IFACE,
"DefaultAdapter",
DBUS_TYPE_INVALID);
bool success = connection->SendWithReply(OnDefaultAdapterReply, nullptr,
1000, "/",
DBUS_MANAGER_IFACE,
"DefaultAdapter",
DBUS_TYPE_INVALID);
if (!success) {
BT_WARNING("Failed to query default adapter!");
}
@ -1834,15 +1811,16 @@ BluetoothDBusService::StopInternal()
}
}
if (!mConnection) {
StopDBus();
RawDBusConnection* connection = GetDBusConnection();
if (!connection) {
return NS_OK;
}
DBusError err;
dbus_error_init(&err);
for (uint32_t i = 0; i < ArrayLength(sBluetoothDBusSignals); ++i) {
dbus_bus_remove_match(mConnection->GetConnection(),
dbus_bus_remove_match(connection->GetConnection(),
sBluetoothDBusSignals[i],
&err);
if (dbus_error_is_set(&err)) {
@ -1850,24 +1828,21 @@ BluetoothDBusService::StopInternal()
}
}
dbus_connection_remove_filter(mConnection->GetConnection(),
dbus_connection_remove_filter(connection->GetConnection(),
EventFilter, nullptr);
if (!dbus_connection_unregister_object_path(gThreadConnection->GetConnection(),
if (!dbus_connection_unregister_object_path(connection->GetConnection(),
KEY_LOCAL_AGENT)) {
BT_WARNING("%s: Can't unregister object path %s for agent!",
__FUNCTION__, KEY_LOCAL_AGENT);
}
if (!dbus_connection_unregister_object_path(gThreadConnection->GetConnection(),
if (!dbus_connection_unregister_object_path(connection->GetConnection(),
KEY_REMOTE_AGENT)) {
BT_WARNING("%s: Can't unregister object path %s for agent!",
__FUNCTION__, KEY_REMOTE_AGENT);
}
mConnection = nullptr;
gThreadConnection = nullptr;
// unref stored DBusMessages before clear the hashtable
sPairingReqTable->EnumerateRead(UnrefDBusMessages, nullptr);
sPairingReqTable->Clear();
@ -1951,14 +1926,14 @@ protected:
// Acquire another reference to this reply handler
nsRefPtr<DefaultAdapterPathReplyHandler> handler = this;
nsRefPtr<RawDBusConnection> threadConnection = gThreadConnection;
RawDBusConnection* connection = GetDBusConnection();
if (!threadConnection.get()) {
if (!connection) {
aReplyError = NS_LITERAL_STRING("DBus connection has been closed.");
return false;
}
bool success = threadConnection->SendWithReply(
bool success = connection->SendWithReply(
DefaultAdapterPathReplyHandler::Callback, handler.get(), 1000,
NS_ConvertUTF16toUTF8(mAdapterPath).get(),
DBUS_ADAPTER_IFACE, "GetProperties", DBUS_TYPE_INVALID);
@ -2019,7 +1994,10 @@ BluetoothDBusService::GetDefaultAdapterPathInternal(
nsRefPtr<DefaultAdapterPathReplyHandler> handler =
new DefaultAdapterPathReplyHandler(aRunnable);
bool success = mConnection->SendWithReply(
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
bool success = connection->SendWithReply(
DefaultAdapterPathReplyHandler::Callback,
handler.get(), 1000,
"/", DBUS_MANAGER_IFACE, "DefaultAdapter",
@ -2064,7 +2042,10 @@ BluetoothDBusService::SendDiscoveryMessage(const char* aMessageName,
nsRefPtr<BluetoothReplyRunnable> runnable(aRunnable);
bool success = mConnection->SendWithReply(
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
bool success = connection->SendWithReply(
OnSendDiscoveryMessageReply,
static_cast<void*>(aRunnable), -1,
NS_ConvertUTF16toUTF8(sAdapterPath).get(),
@ -2104,13 +2085,12 @@ BluetoothDBusService::SendAsyncDBusMessage(const nsAString& aObjectPath,
DBusReplyCallback aCallback)
{
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(mConnection);
MOZ_ASSERT(IsEnabled());
MOZ_ASSERT(aCallback);
MOZ_ASSERT(!aObjectPath.IsEmpty());
MOZ_ASSERT(aInterface);
NS_ENSURE_TRUE(mConnection, NS_ERROR_FAILURE);
NS_ENSURE_TRUE(GetDBusConnection(), NS_ERROR_FAILURE);
nsAutoPtr<BluetoothServiceClass> serviceClass(new BluetoothServiceClass());
if (!strcmp(aInterface, DBUS_SINK_IFACE)) {
@ -2122,7 +2102,10 @@ BluetoothDBusService::SendAsyncDBusMessage(const nsAString& aObjectPath,
return NS_ERROR_FAILURE;
}
bool ret = mConnection->SendWithReply(
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
bool ret = connection->SendWithReply(
aCallback, static_cast<void*>(serviceClass.forget()), -1,
NS_ConvertUTF16toUTF8(aObjectPath).get(),
aInterface, NS_ConvertUTF16toUTF8(aMessage).get(),
@ -2267,16 +2250,16 @@ protected:
mObjectPath = GetObjectPathFromAddress(sAdapterPath,
mDeviceAddresses[mProcessedDeviceAddresses]);
nsRefPtr<RawDBusConnection> threadConnection = gThreadConnection;
RawDBusConnection* connection = GetDBusConnection();
if (!threadConnection.get()) {
if (!connection) {
BT_WARNING("%s: DBus connection has been closed.", __FUNCTION__);
return false;
}
nsRefPtr<BluetoothArrayOfDevicePropertiesReplyHandler> handler = this;
bool success = threadConnection->SendWithReply(
bool success = connection->SendWithReply(
BluetoothArrayOfDevicePropertiesReplyHandler::Callback,
handler.get(), 1000,
NS_ConvertUTF16toUTF8(mObjectPath).get(),
@ -2435,10 +2418,13 @@ BluetoothDBusService::SetProperty(BluetoothObjectType aType,
nsRefPtr<BluetoothReplyRunnable> runnable = aRunnable;
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
// msg is unref'd as part of SendWithReply
bool success = mConnection->SendWithReply(GetVoidCallback,
(void*)aRunnable,
1000, msg);
bool success = connection->SendWithReply(GetVoidCallback,
(void*)aRunnable,
1000, msg);
if (!success) {
BT_WARNING("SendWithReply failed");
return NS_ERROR_FAILURE;
@ -2476,9 +2462,12 @@ BluetoothDBusService::CreatePairedDeviceInternal(
nsRefPtr<BluetoothReplyRunnable> runnable = aRunnable;
MOZ_ASSERT(!sAdapterPath.IsEmpty());
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
// Then send CreatePairedDevice, it will register a temp device agent then
// unregister it after pairing process is over
bool ret = mConnection->SendWithReply(
bool ret = connection->SendWithReply(
GetObjectPathCallback, (void*)runnable, aTimeout,
NS_ConvertUTF16toUTF8(sAdapterPath).get(),
DBUS_ADAPTER_IFACE,
@ -2531,7 +2520,10 @@ BluetoothDBusService::RemoveDeviceInternal(const nsAString& aDeviceAddress,
nsRefPtr<BluetoothReplyRunnable> runnable(aRunnable);
bool success = mConnection->SendWithReply(
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
bool success = connection->SendWithReply(
OnRemoveDeviceReply, static_cast<void*>(runnable.get()), -1,
NS_ConvertUTF16toUTF8(sAdapterPath).get(),
DBUS_ADAPTER_IFACE, "RemoveDevice",
@ -2582,7 +2574,9 @@ BluetoothDBusService::SetPinCodeInternal(const nsAString& aDeviceAddress,
errorStr.AssignLiteral("Couldn't append arguments to dbus message.");
result = false;
} else {
result = mConnection->Send(reply);
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
result = connection->Send(reply);
}
dbus_message_unref(msg);
@ -2628,7 +2622,9 @@ BluetoothDBusService::SetPasskeyInternal(const nsAString& aDeviceAddress,
errorStr.AssignLiteral("Couldn't append arguments to dbus message.");
result = false;
} else {
result = mConnection->Send(reply);
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
result = connection->Send(reply);
}
dbus_message_unref(msg);
@ -2672,7 +2668,10 @@ BluetoothDBusService::SetPairingConfirmationInternal(
return false;
}
bool result = mConnection->Send(reply);
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
bool result = connection->Send(reply);
if (!result) {
errorStr.AssignLiteral("Can't send message!");
}
@ -2915,7 +2914,10 @@ BluetoothDBusService::GetServiceChannel(const nsAString& aDeviceAddress,
nsRefPtr<OnGetServiceChannelReplyHandler> handler =
new OnGetServiceChannelReplyHandler(objectPath, aServiceUUID, aManager);
bool success = mConnection->SendWithReply(
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
bool success = connection->SendWithReply(
OnGetServiceChannelReplyHandler::Callback, handler, -1,
NS_ConvertUTF16toUTF8(objectPath).get(),
DBUS_DEVICE_IFACE, "GetServiceAttributeValue",
@ -2960,7 +2962,6 @@ BluetoothDBusService::UpdateSdpRecords(const nsAString& aDeviceAddress,
MOZ_ASSERT(!aDeviceAddress.IsEmpty());
MOZ_ASSERT(!sAdapterPath.IsEmpty());
MOZ_ASSERT(aManager);
MOZ_ASSERT(mConnection);
nsString objectPath(GetObjectPathFromAddress(sAdapterPath, aDeviceAddress));
@ -2969,13 +2970,16 @@ BluetoothDBusService::UpdateSdpRecords(const nsAString& aDeviceAddress,
OnUpdateSdpRecordsRunnable* callbackRunnable =
new OnUpdateSdpRecordsRunnable(objectPath, aManager);
return mConnection->SendWithReply(DiscoverServicesCallback,
(void*)callbackRunnable, -1,
NS_ConvertUTF16toUTF8(objectPath).get(),
DBUS_DEVICE_IFACE,
"DiscoverServices",
DBUS_TYPE_STRING, &EmptyCString(),
DBUS_TYPE_INVALID);
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
return connection->SendWithReply(DiscoverServicesCallback,
(void*)callbackRunnable, -1,
NS_ConvertUTF16toUTF8(objectPath).get(),
DBUS_DEVICE_IFACE,
"DiscoverServices",
DBUS_TYPE_STRING, &EmptyCString(),
DBUS_TYPE_INVALID);
}
void
@ -3152,7 +3156,10 @@ BluetoothDBusService::SendMetaData(const nsAString& aTitle,
nsRefPtr<BluetoothReplyRunnable> runnable(aRunnable);
bool ret = mConnection->SendWithReply(
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
bool ret = connection->SendWithReply(
GetVoidCallback, (void*)runnable.get(), -1,
NS_ConvertUTF16toUTF8(objectPath).get(),
DBUS_CTL_IFACE, "UpdateMetaData",
@ -3251,7 +3258,10 @@ BluetoothDBusService::SendPlayStatus(int64_t aDuration,
nsRefPtr<BluetoothReplyRunnable> runnable(aRunnable);
bool ret = mConnection->SendWithReply(
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
bool ret = connection->SendWithReply(
GetVoidCallback, (void*)runnable.get(), -1,
NS_ConvertUTF16toUTF8(objectPath).get(),
DBUS_CTL_IFACE, "UpdatePlayStatus",
@ -3300,7 +3310,10 @@ BluetoothDBusService::UpdatePlayStatus(uint32_t aDuration,
uint32_t tempPlayStatus = aPlayStatus;
bool ret = mConnection->SendWithReply(
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
bool ret = connection->SendWithReply(
ControlCallback, nullptr, -1,
NS_ConvertUTF16toUTF8(objectPath).get(),
DBUS_CTL_IFACE, "UpdatePlayStatus",
@ -3330,7 +3343,10 @@ BluetoothDBusService::UpdateNotification(ControlEventId aEventId,
GetObjectPathFromAddress(sAdapterPath, address);
uint16_t eventId = aEventId;
bool ret = mConnection->SendWithReply(
RawDBusConnection* connection = GetDBusConnection();
MOZ_ASSERT(connection);
bool ret = connection->SendWithReply(
ControlCallback, nullptr, -1,
NS_ConvertUTF16toUTF8(objectPath).get(),
DBUS_CTL_IFACE, "UpdateNotification",

View File

@ -194,8 +194,6 @@ private:
const char* aInterface,
const nsAString& aMessage,
mozilla::ipc::DBusReplyCallback aCallback);
nsRefPtr<mozilla::ipc::RawDBusConnection> mConnection;
};
END_BLUETOOTH_NAMESPACE

View File

@ -75,10 +75,14 @@ class DBusWatcher
{
public:
DBusWatcher()
: mConnection(nullptr)
{ }
~DBusWatcher()
{ }
{
// Connection has been released
MOZ_ASSERT(!mConnection);
}
bool Initialize();
void CleanUp();
@ -94,6 +98,8 @@ public:
void HandleWatchAdd();
void HandleWatchRemove();
RawDBusConnection* GetConnection();
private:
struct PollFdComparator {
bool Equals(const pollfd& a, const pollfd& b) const {
@ -132,9 +138,15 @@ private:
ScopedClose mControlFdR;
ScopedClose mControlFdW;
nsRefPtr<RawDBusConnection> mConnection;
RawDBusConnection* mConnection;
};
RawDBusConnection*
DBusWatcher::GetConnection()
{
return mConnection;
}
bool
DBusWatcher::Initialize()
{
@ -175,6 +187,9 @@ DBusWatcher::CleanUp()
// DBusWatch pointers are maintained by DBus, so we won't leak by
// clearing.
mWatchData.Clear();
delete mConnection;
mConnection = nullptr;
}
void
@ -516,7 +531,7 @@ DBusWatcher::SetUp()
mWatchData.AppendElement(static_cast<DBusWatch*>(nullptr));
nsRefPtr<RawDBusConnection> connection = new RawDBusConnection();
RawDBusConnection* connection = new RawDBusConnection();
// If we can't establish a connection to dbus, nothing else will work
nsresult rv = connection->EstablishDBusConnection();
@ -652,5 +667,13 @@ DispatchToDBusThread(nsIRunnable* event)
return NS_OK;
}
RawDBusConnection*
GetDBusConnection()
{
NS_ENSURE_TRUE(gDBusWatcher, nullptr);
return gDBusWatcher->GetConnection();
}
}
}

View File

@ -14,6 +14,8 @@ class nsIRunnable;
namespace mozilla {
namespace ipc {
class RawDBusConnection;
/**
* Starts the DBus thread, which handles returning signals to objects
* that call asynchronous functions. This should be called from the
@ -40,6 +42,9 @@ bool StopDBus();
nsresult
DispatchToDBusThread(nsIRunnable* event);
RawDBusConnection*
GetDBusConnection(void);
}
}

View File

@ -14,7 +14,6 @@
#include <string>
#include "nscore.h"
#include "mozilla/Scoped.h"
#include <mozilla/RefPtr.h>
#include <mozilla/Mutex.h>
struct DBusConnection;
@ -26,7 +25,7 @@ namespace ipc {
typedef void (*DBusReplyCallback)(DBusMessage*, void*);
class RawDBusConnection : public AtomicRefCounted<RawDBusConnection>
class RawDBusConnection
{
struct ScopedDBusConnectionPtrTraits : ScopedFreePtrTraits<DBusConnection>
{