Bug 1158818: Only store Bluetooth result runnable after command has been sent successfully, r=shuang
authorThomas Zimmermann <tdz@users.sourceforge.net>
Thu, 30 Apr 2015 11:29:25 +0200
changeset 273042 a67e86912144c715f268ad873a3b0e9faaef090b
parent 273041 5ae1136d4e3bc29441ff31b05a9fb4ed74deb298
child 273043 2f7d116f782c75ba3d1d70350eff4f3ed711fe23
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshuang
bugs1158818
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1158818: Only store Bluetooth result runnable after command has been sent successfully, r=shuang With the current code, the Bluetooth result runnable is saved for receiving before a command has been sent successfully. If sending fails afterwards, the saved result runnable will still sit in the result queue, and interfere with later, successful commands. With this patch, the result runnable is saved only if the sending was successful.
dom/bluetooth/bluedroid/BluetoothDaemonInterface.cpp
ipc/bluetooth/BluetoothDaemonConnection.cpp
ipc/bluetooth/BluetoothDaemonConnection.h
--- a/dom/bluetooth/bluedroid/BluetoothDaemonInterface.cpp
+++ b/dom/bluetooth/bluedroid/BluetoothDaemonInterface.cpp
@@ -1502,16 +1502,17 @@ BluetoothDaemonProtocol::UnregisterModul
 }
 
 nsresult
 BluetoothDaemonProtocol::Send(BluetoothDaemonPDU* aPDU, void* aUserData)
 {
   MOZ_ASSERT(mConnection);
   MOZ_ASSERT(aPDU);
 
+  aPDU->SetConsumer(this);
   aPDU->SetUserData(aUserData);
   aPDU->UpdateHeader();
   return mConnection->Send(aPDU); // Forward PDU to command channel
 }
 
 void
 BluetoothDaemonProtocol::HandleSetupSvc(
   const BluetoothDaemonPDUHeader& aHeader, BluetoothDaemonPDU& aPDU,
--- a/ipc/bluetooth/BluetoothDaemonConnection.cpp
+++ b/ipc/bluetooth/BluetoothDaemonConnection.cpp
@@ -37,31 +37,33 @@ namespace ipc {
 static const char sBluetoothdSocketName[] = "bluez_hal_socket";
 
 //
 // BluetoothDaemonPDU
 //
 
 BluetoothDaemonPDU::BluetoothDaemonPDU(uint8_t aService, uint8_t aOpcode,
                                        uint16_t aPayloadSize)
-: UnixSocketIOBuffer(HEADER_SIZE + aPayloadSize)
-, mUserData(nullptr)
+  : UnixSocketIOBuffer(HEADER_SIZE + aPayloadSize)
+  , mConsumer(nullptr)
+  , mUserData(nullptr)
 {
   uint8_t* data = Append(HEADER_SIZE);
   MOZ_ASSERT(data);
 
   // Setup PDU header
   data[OFF_SERVICE] = aService;
   data[OFF_OPCODE] = aOpcode;
   memcpy(data + OFF_LENGTH, &aPayloadSize, sizeof(aPayloadSize));
 }
 
 BluetoothDaemonPDU::BluetoothDaemonPDU(size_t aPayloadSize)
-: UnixSocketIOBuffer(HEADER_SIZE + aPayloadSize)
-, mUserData(nullptr)
+  : UnixSocketIOBuffer(HEADER_SIZE + aPayloadSize)
+  , mConsumer(nullptr)
+  , mUserData(nullptr)
 { }
 
 void
 BluetoothDaemonPDU::GetHeader(uint8_t& aService, uint8_t& aOpcode,
                               uint16_t& aPayloadSize)
 {
   memcpy(&aService, GetData(OFF_SERVICE), sizeof(aService));
   memcpy(&aOpcode, GetData(OFF_OPCODE), sizeof(aOpcode));
@@ -87,16 +89,22 @@ BluetoothDaemonPDU::Send(int aFd)
   if (res < 0) {
     MOZ_ASSERT(errno != EBADF); /* internal error */
     OnError("sendmsg", errno);
     return -1;
   }
 
   Consume(res);
 
+  if (mConsumer) {
+    // We successfully sent a PDU, now store the
+    // result runnable in the consumer.
+    mConsumer->StoreUserData(*this);
+  }
+
   return res;
 }
 
 #define CMSGHDR_CONTAINS_FD(_cmsghdr) \
     ( ((_cmsghdr)->cmsg_level == SOL_SOCKET) && \
       ((_cmsghdr)->cmsg_type == SCM_RIGHTS) )
 
 ssize_t
@@ -371,17 +379,16 @@ BluetoothDaemonConnectionIO::Connect(con
 }
 
 void
 BluetoothDaemonConnectionIO::Send(BluetoothDaemonPDU* aPDU)
 {
   MOZ_ASSERT(mConsumer);
   MOZ_ASSERT(aPDU);
 
-  mConsumer->StoreUserData(*aPDU); // Store user data for reply
   EnqueueData(aPDU);
   AddWatchers(WRITE_WATCHER, false);
 }
 
 void
 BluetoothDaemonConnectionIO::EnqueueData(BluetoothDaemonPDU* aPDU)
 {
   MOZ_ASSERT(aPDU);
--- a/ipc/bluetooth/BluetoothDaemonConnection.h
+++ b/ipc/bluetooth/BluetoothDaemonConnection.h
@@ -13,16 +13,17 @@
 #include "mozilla/ipc/SocketBase.h"
 #include "nsError.h"
 #include "nsAutoPtr.h"
 
 namespace mozilla {
 namespace ipc {
 
 class BluetoothDaemonConnectionIO;
+class BluetoothDaemonPDUConsumer;
 
 /*
  * |BlutoothDaemonPDU| represents a single PDU that is transfered from or to
  * the Bluetooth daemon. Each PDU contains exactly one command.
  *
  * A PDU as the following format
  *
  *    |    1    |    1    |        2       |    n    |
@@ -50,16 +51,21 @@ public:
     HEADER_SIZE = OFF_PAYLOAD,
     MAX_PAYLOAD_LENGTH = 1 << 16
   };
 
   BluetoothDaemonPDU(uint8_t aService, uint8_t aOpcode,
                      uint16_t aPayloadSize);
   BluetoothDaemonPDU(size_t aPayloadSize);
 
+  void SetConsumer(BluetoothDaemonPDUConsumer* aConsumer)
+  {
+    mConsumer = aConsumer;
+  }
+
   void SetUserData(void* aUserData)
   {
     mUserData = aUserData;
   }
 
   void* GetUserData() const
   {
     return mUserData;
@@ -74,16 +80,17 @@ public:
   int AcquireFd();
 
   nsresult UpdateHeader();
 
 private:
   size_t GetPayloadSize() const;
   void OnError(const char* aFunction, int aErrno);
 
+  BluetoothDaemonPDUConsumer* mConsumer;
   void* mUserData;
   ScopedClose mReceivedFd;
 };
 
 /*
  * |BluetoothDaemonPDUConsumer| processes incoming PDUs from the Bluetooth
  * daemon. Please note that its method |Handle| runs on a different than the
  * main thread.