Changeset 49570 in vbox

Timestamp:

Nov 20, 2013 9:13:04 AM (11 years ago)

Author:

vboxsync

Message:

Main/GuestProcessImpl.cpp: Don't use AssertReleaseMsg() in host callbacks, also check for stale guest processes in protocol v2, unified guestRc handling in GuestProcessTool::RunEx().

Location:

trunk/src/VBox/Main/src-client

Files:

• GuestProcessImpl.cpp (modified) (7 diffs)
• GuestSessionImpl.cpp (modified) (10 diffs)

trunk/src/VBox/Main/src-client/GuestProcessImpl.cpp

@@ -519 +519 @@
  * a process B. Process B in turn then has a different guest PID.
  *
+ * Note: This also can happen when restoring from a saved state which
+ *       had a guest process running.
+ *
  * @return  IPRT status code.
  * @param   uPID                    PID to check.
@@ -524 +527 @@
 inline int GuestProcess::checkPID(uint32_t uPID)
 {
+    int rc = VINF_SUCCESS;
+
     /* Was there a PID assigned yet? */
     if (mData.mPID)
     {
-        /*
-
-         */
-        if (mSession->getProtocolVersion() < 2)
-        {
-            /* Simply ignore the stale requests. */
-            return (mData.mPID == uPID)
-                   ? VINF_SUCCESS : VERR_NOT_FOUND;
-        }
-#ifndef DEBUG_andy
-        /* This should never happen! */
-        AssertReleaseMsg(mData.mPID == uPID, ("Unterminated guest process (guest PID %RU32) sent data to a newly started process (host PID %RU32)\n",
-                                              uPID, mData.mPID));
-#endif
-    }
-
-    return VINF_SUCCESS;
+        if (RT_UNLIKELY(mData.mPID != uPID))
+        {
+            LogFlowFunc(("Stale guest process (PID=%RU32) sent data to a newly started process (pProcesS=%p, PID=%RU32, status=%RU32)\n",
+                         uPID, this, mData.mPID, mData.mStatus));
+            rc = VERR_NOT_FOUND;
+        }
+    }
+
+    return rc;
 }
 
@@ -865 +862 @@
 
 /**
- * Called by IGuestSession right before this process gets 
- * removed from the public process list. 
+ * Called by IGuestSession right before this process gets
+ * removed from the public process list.
  */
 int GuestProcess::onRemove(void)
@@ -876 +873 @@
     int vrc = VINF_SUCCESS;
 
-    /* 
+    /*
      * Note: The event source stuff holds references to this object,
      *       so make sure that this is cleaned up *before* calling uninit().
@@ -2060 +2057 @@
     int vrc = pSession->processCreateExInteral(mStartupInfo, pProcess);
     if (RT_SUCCESS(vrc))
-        vrc = fAsync 
-            ? pProcess->startProcessAsync() 
+        vrc = fAsync
+            ? pProcess->startProcessAsync()
             : pProcess->startProcess(30 * 1000 /* 30s timeout */, pGuestRc);
 
@@ -2164 +2161 @@
         /* Make sure the process runs until completion. */
         vrc = procTool.Wait(GUESTPROCESSTOOL_FLAG_NONE, &guestRc);
-
-        Assert(RT_SUCCESS(guestRc));
-        guestRc = procTool.TerminatedOk(NULL /* Exit code */);
-        if (RT_FAILURE(guestRc))
-            vrc = VERR_GSTCTL_GUEST_ERROR;                        
-        if (pGuestRc)
-            *pGuestRc = guestRc;
-    }
-    else if (vrc == VERR_GSTCTL_GUEST_ERROR)
-    {
-        if (pGuestRc)
-            *pGuestRc = guestRc;
-    }
+        if (RT_SUCCESS(vrc))
+        {
+            guestRc = procTool.TerminatedOk(NULL /* Exit code */);
+            if (RT_FAILURE(guestRc))
+                vrc = VERR_GSTCTL_GUEST_ERROR;
+        }
+    }
+
+    if (pGuestRc)
+        *pGuestRc = guestRc;
 
     LogFlowFunc(("Returned rc=%Rrc, guestRc=%Rrc\n", vrc, guestRc));
@@ -2322 +2316 @@
 
             default:
-                AssertReleaseMsgFailed(("Unhandled process wait result %RU32\n", waitRes));
+                AssertMsgFailed(("Unhandled process wait result %RU32\n", waitRes));
                 break;
         }

trunk/src/VBox/Main/src-client/GuestSessionImpl.cpp

@@ -283 +283 @@
 
 #ifdef VBOX_WITH_GUEST_CONTROL
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); 
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
     LogFlowThisFunc(("Closing directories (%zu total)\n",
@@ -321 +321 @@
     mData.mProcesses.clear();
 
-    AssertMsg(mData.mNumObjects == 0, 
+    AssertMsg(mData.mNumObjects == 0,
               ("mNumObjects=%RU32 when it should be 0\n", mData.mNumObjects));
 
@@ -731 +731 @@
                 vrc = VERR_INVALID_PARAMETER;
         }
-        
+
         if (uMode)
         {
@@ -808 +808 @@
             LogFlowFunc(("Removing directory \"%s\" (Session: %RU32) (now total %zu processes, %ld objects)\n",
                          Utf8Str(strName).c_str(), mData.mSession.mID, mData.mDirectories.size() - 1, mData.mNumObjects - 1));
- 
+
             rc = pDirectory->onRemove();
             mData.mDirectories.erase(itDirs);
@@ -1194 +1194 @@
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
-    int rc = VERR_NOT_FOUND;    
+    int rc = VERR_NOT_FOUND;
 
     SessionFiles::iterator itFiles = mData.mFiles.begin();
@@ -1221 +1221 @@
             fireGuestFileRegisteredEvent(mEventSource, this, pCurFile,
                                          false /* Unregistered */);
-            pCurFile.setNull();            
+            pCurFile.setNull();
             break;
         }
@@ -1229 +1229 @@
 
     LogFlowFuncLeaveRC(rc);
-    return rc;    
+    return rc;
 }
 
@@ -1426 +1426 @@
     }
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc, guestRc=%Rrc\n",
+                     vrc, guestRc));
     return vrc;
 }
@@ -1515 +1516 @@
 
 /**
- * Called by IGuest right before this session gets removed from 
- * the public session list. 
+ * Called by IGuest right before this session gets removed from
+ * the public session list.
  */
 int GuestSession::onRemove(void)
@@ -1526 +1527 @@
     int vrc = VINF_SUCCESS;
 
-    /* 
+    /*
      * Note: The event source stuff holds references to this object,
      *       so make sure that this is cleaned up *before* calling uninit.