Changeset 26554 in vbox

Timestamp:

Feb 15, 2010 6:13:54 PM (15 years ago)

Author:

vboxsync

Message:

XPCOM: fix IPC message processing in the situation where several threads call the same client which then crashes. Before only one thread got the client death message, and the others would hang forever. Essentially added code which keeps track of the client state.

Location:

trunk/src/libs/xpcom18a4/ipc/ipcd

Files:

• client/src/ipcdclient.cpp (modified) (6 diffs)
• extensions/dconnect/src/ipcDConnectService.cpp (modified) (3 diffs)

trunk/src/libs/xpcom18a4/ipc/ipcd/client/src/ipcdclient.cpp

@@ -43 +43 @@
 #include "ipcLog.h"
 #include "ipcm.h"
+#include "iprt/thread.h"
 
 #include "nsIFile.h"
@@ -405 +406 @@
       break;
     }
+#ifdef VBOX
+    else
+    {
+      /* Special client liveness check if there is no message to process.
+       * This is necessary as there might be several threads waiting for
+       * a message from a single client, and only one gets the DOWN msg. */
+      PRBool alive = (aSelector)(aArg, td, NULL);
+      if (!alive)
+      {
+        *aMsg = NULL;
+        break;
+      }
+    }
+#endif /* VBOX */
 
     PRIntervalTime t = PR_IntervalNow();
@@ -587 +602 @@
 WaitIPCMResponseSelector(void *arg, ipcTargetData *td, const ipcMessage *msg)
 {
+#ifdef VBOX
+  if (!msg)
+    return PR_TRUE;
+#endif /* VBOX */
   PRUint32 requestIndex = *(PRUint32 *) arg;
   return IPCM_GetRequestIndex(msg) == requestIndex;
@@ -914 +933 @@
 {
   WaitMessageSelectorData *data = (WaitMessageSelectorData *) arg;
+#ifdef VBOX
+  if (!msg)
+  {
+    /* Special NULL message which asks to check whether the client is
+     * still alive. Called when there is nothing suitable in the queue. */
+    ipcIMessageObserver *obs = data->observer;
+    if (!obs)
+      obs = td->observer;
+    NS_ASSERTION(obs, "must at least have a default observer");
+
+    nsresult rv = obs->OnMessageAvailable(IPC_SENDER_ANY, nsID(), 0, 0);
+    if (rv != IPC_WAIT_NEXT_MESSAGE)
+    {
+      data->senderDead = PR_TRUE;
+      return PR_FALSE;
+    }
+    return PR_TRUE;
+  }
+#endif /* VBOX */
 
   // process the specially forwarded client state message to see if the
@@ -943 +981 @@
           {
             // otherwise inform the observer about the client death using a special
-            // null message with an emply target id, and fail IPC_WaitMessage call
+            // null message with an empty target id, and fail IPC_WaitMessage call
             // with NS_ERROR_xxx only if the observer accepts this message.
 
@@ -959 +997 @@
           }
         }
+#ifdef VBOX
+        else if ((data->senderID == IPC_SENDER_ANY ||
+                  status->ClientID() == data->senderID) &&
+                 status->ClientState() == IPCM_CLIENT_STATE_UP)
+        {
+          LOG(("sender (%d) we're waiting a message from (%d) has come up\n",
+               status->ClientID(), data->senderID));
+          if (data->senderID == IPC_SENDER_ANY)
+          {
+            // inform the observer about the client appearance using a special
+            // null message with an empty target id, but a length of 1.
+
+            ipcIMessageObserver *obs = data->observer;
+            if (!obs)
+              obs = td->observer;
+            NS_ASSERTION(obs, "must at least have a default observer");
+
+            nsresult rv = obs->OnMessageAvailable(status->ClientID(), nsID(), 0, 1);
+            if (rv != IPC_WAIT_NEXT_MESSAGE)
+            {
+              /* It might sound a bit paradoxical to declare the sender as
+               * dead, but the fact that a client up message is received
+               * while waiting for a message from this client clearly
+               * indicates that this is no longer the client we were waiting
+               * for, but a new one got the same client ID (due to wraparound).
+               * Shouldn't happen in real life, but better be safe. */
+              data->senderDead = PR_TRUE;
+              return PR_TRUE; // consume the message
+            }
+          }
+        }
+#endif /* VBOX */
         break;
       }

trunk/src/libs/xpcom18a4/ipc/ipcd/extensions/dconnect/src/ipcDConnectService.cpp

@@ -51 +51 @@
 #include "nsDeque.h"
 #include "xptcall.h"
+
+#ifdef VBOX
+#include <map>
+#include <list>
+#endif /* VBOX */
 
 #if defined(DCONNECT_MULTITHREADED)
@@ -1255 +1260 @@
 //-----------------------------------------------------------------------------
 
+#ifdef VBOX
+typedef struct ClientDownInfo
+{
+    ClientDownInfo(PRUint32 aClient)
+    {
+        uClient = aClient;
+        uTimestamp = PR_IntervalNow();
+    }
+
+    PRUint32 uClient;
+    PRIntervalTime uTimestamp;
+} ClientDownInfo;
+typedef std::map<PRUint32, ClientDownInfo *> ClientDownMap;
+typedef std::list<ClientDownInfo *> ClientDownList;
+
+#define MAX_CLIENT_DOWN_SIZE 10000
+
+/* Protected by the queue monitor. */
+static ClientDownMap g_ClientDownMap;
+static ClientDownList g_ClientDownList;
+
+#endif /* VBOX */
+
 class DConnectMsgSelector : public ipcIMessageObserver
 {
@@ -1275 +1303 @@
     // accept special "client dead" messages for a given peer
     // (empty target id, zero data and data length)
+#ifndef VBOX
     if (aSenderID == mPeer && aTarget.Equals(nsID()) && !aData && !aDataLen)
         return NS_OK;
+#else /* VBOX */
+    if (aSenderID != IPC_SENDER_ANY && aTarget.Equals(nsID()) && !aData && !aDataLen)
+    {
+        // Insert new client down information. Start by expiring outdated
+        // entries and free one element if there's still no space (if needed).
+        PRIntervalTime now = PR_IntervalNow();
+        do {
+            ClientDownInfo *cInfo = g_ClientDownList.back();
+            if (!cInfo)
+                break;
+            PRInt64 diff = (PRInt64)now - cInfo->uTimestamp;
+            if (diff < 0)
+                diff += (PRInt64)((PRIntervalTime)-1) + 1;
+            if (diff > PR_SecondsToInterval(15 * 60))
+            {
+                g_ClientDownMap.erase(cInfo->uClient);
+                g_ClientDownList.pop_back();
+                delete cInfo;
+            }
+            else
+                break;
+        } while (true);
+
+        ClientDownMap::iterator it = g_ClientDownMap.find(aSenderID);
+        if (it == g_ClientDownMap.end())
+        {
+            while (g_ClientDownList.size() >= MAX_CLIENT_DOWN_SIZE)
+            {
+                ClientDownInfo *cInfo = g_ClientDownList.back();
+                g_ClientDownMap.erase(cInfo->uClient);
+                g_ClientDownList.pop_back();
+                delete cInfo;
+            }
+
+            ClientDownInfo *cInfo = new ClientDownInfo(aSenderID);
+            g_ClientDownMap[aSenderID] = cInfo;
+            g_ClientDownList.push_front(cInfo);
+        }
+        return (aSenderID == mPeer) ? NS_OK : IPC_WAIT_NEXT_MESSAGE;
+    }
+    // accept special "client up" messages for a given peer
+    // (empty target id, zero data and data length=1)
+    if (aTarget.Equals(nsID()) && !aData && aDataLen == 1)
+    {
+        ClientDownMap::iterator it = g_ClientDownMap.find(aSenderID);
+        if (it != g_ClientDownMap.end())
+        {
+            ClientDownInfo *cInfo = it->second;
+            g_ClientDownMap.erase(it);
+            g_ClientDownList.remove(cInfo);
+            delete cInfo;
+        }
+        return (aSenderID == mPeer) ? NS_OK : IPC_WAIT_NEXT_MESSAGE;
+    }
+    // accept special "client check" messages for an anonymous sender
+    // (invalid sender id, empty target id, zero data and data length
+    if (aSenderID == IPC_SENDER_ANY && aTarget.Equals(nsID()) && !aData && !aDataLen)
+    {
+        LOG(("DConnectMsgSelector::OnMessageAvailable: poll liveness for mPeer=%d\n",
+             mPeer));
+        ClientDownMap::iterator it = g_ClientDownMap.find(mPeer);
+        return (it == g_ClientDownMap.end()) ? IPC_WAIT_NEXT_MESSAGE : NS_OK;
+    }
+#endif /* VBOX */
     const DConnectOp *op = (const DConnectOp *) aData;
     // accept only reply messages with the given peer/opcode/index