Changeset 26582 in vbox

Timestamp:

Feb 16, 2010 3:01:50 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. r57733 was too paranoid, and failed more or less frequently when VBoxSVC was autostarted.

Location:

trunk/src/libs/xpcom18a4/ipc/ipcd

Files:

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

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

@@ -405 +405 @@
       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 +601 @@
 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 +932 @@
 {
   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 +980 @@
           {
             // 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 +996 @@
           }
         }
+#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);
+            /* VBoxSVC/VBoxXPCOMIPCD auto-start can cause that a client up
+             * message arrives while we're already waiting for a response
+             * from this client. Don't declare the connection as dead in
+             * this case. A client ID wraparound can't falsely trigger
+             * this, since the waiting thread would have hit the liveness
+             * check in the mean time. Also, DO NOT consume the message,
+             * otherwise it gets passed to the DConnect message handling
+             * without any further checks. IPCM messages are automatically
+             * discarded if they are left unclaimed. */
+          }
+        }
+#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