Changeset 28943 in vbox

Timestamp:

Apr 30, 2010 3:47:23 PM (15 years ago)

Author:

vboxsync

Message:

Guest Control/Main/VBoxService: Update on locking, bugfixes.

Location:

trunk/src/VBox

Files:

• Additions/common/VBoxService/VBoxServiceControlExec.cpp (modified) (4 diffs)
• Additions/common/VBoxService/VBoxServiceInternal.h (modified) (2 diffs)
• Frontends/VBoxManage/VBoxManageGuestCtrl.cpp (modified) (3 diffs)
• Main/GuestImpl.cpp (modified) (3 diffs)

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceControlExec.cpp

@@ -535 +535 @@
 }
 
+int VBoxServiceControlExecInitPipeBuffer(PVBOXSERVICECTRLEXECPIPEBUF pBuf)
+{
+    AssertPtr(pBuf);
+
+    pBuf->pbData = (uint8_t*)RTMemAlloc(_64K); /* Start with a 64k buffer. */
+    AssertReturn(pBuf->pbData, VERR_NO_MEMORY);
+    pBuf->cbSize = 0;
+    pBuf->cbOffset = 0;
+    pBuf->cbRead = 0;
+
+    return RTSemMutexCreate(&pBuf->mtx);
+}
+
+int VBoxServiceControlExecDestroyPipeBuffer(PVBOXSERVICECTRLEXECPIPEBUF pBuf)
+{
+    if (pBuf)
+    {
+        if (pBuf->pbData)
+            RTMemFree(pBuf->pbData);
+        pBuf->pbData = NULL;
+        pBuf->cbSize = 0;
+        pBuf->cbOffset = 0;
+        pBuf->cbRead = 0;
+    }
+    return RTSemMutexDestroy(pBuf->mtx);
+}
+
 int VBoxServiceControlExecReadPipeBufferContent(PVBOXSERVICECTRLEXECPIPEBUF pBuf,
                                                 uint8_t *pbBuffer, uint32_t cbBuffer, uint32_t *pcbToRead)
 {
+    AssertPtr(pBuf);
     AssertPtr(pcbToRead);
 
-// LOCKING
-
-    Assert(pBuf->cbOffset >= pBuf->cbRead);
-    if (*pcbToRead > pBuf->cbOffset - pBuf->cbRead)
-        *pcbToRead = pBuf->cbOffset - pBuf->cbRead;
-
-    if (*pcbToRead > cbBuffer)
-        *pcbToRead = cbBuffer;
-
-    if (*pcbToRead > 0)
-    {
-        memcpy(pbBuffer, pBuf->pbData + pBuf->cbRead, *pcbToRead);
-        pBuf->cbRead += *pcbToRead;
-    }
-    else
-    {
-        pbBuffer = NULL;
-        *pcbToRead = 0;
-    }
-    return VINF_SUCCESS;
+    int rc = RTSemMutexRequest(pBuf->mtx, RT_INDEFINITE_WAIT);
+    if (RT_SUCCESS(rc))
+    {    
+        Assert(pBuf->cbOffset >= pBuf->cbRead);
+        if (*pcbToRead > pBuf->cbOffset - pBuf->cbRead)
+            *pcbToRead = pBuf->cbOffset - pBuf->cbRead;
+    
+        if (*pcbToRead > cbBuffer)
+            *pcbToRead = cbBuffer;
+    
+        if (*pcbToRead > 0)
+        {
+            memcpy(pbBuffer, pBuf->pbData + pBuf->cbRead, *pcbToRead);
+            pBuf->cbRead += *pcbToRead;
+        }
+        else
+        {
+            pbBuffer = NULL;
+            *pcbToRead = 0;
+        }
+        rc = RTSemMutexRelease(pBuf->mtx);
+    }
+    return rc;
 }
 
@@ -567 +598 @@
     AssertPtr(pBuf);
 
-// LOCKING
-
-    /** @todo Use RTMemCache or RTMemObj here? */
-    uint8_t *pNewBuf;
-    while (pBuf->cbSize - pBuf->cbOffset < cbData)
-    {
-        pNewBuf = (uint8_t*)RTMemRealloc(pBuf->pbData, pBuf->cbSize + _4K);
-        if (pNewBuf == NULL)
-            break;
-        pBuf->cbSize += _4K;
-        pBuf->pbData = pNewBuf;
-    }
-
-    int rc = VINF_SUCCESS;
-    if (pBuf->pbData)
-    {
-        memcpy(pBuf->pbData + pBuf->cbOffset, pbData, cbData);
-        pBuf->cbOffset += cbData;
-        /** @todo Add offset clamping! */
-    }
-    else
-        rc = VERR_NO_MEMORY;
+    int rc = RTSemMutexRequest(pBuf->mtx, RT_INDEFINITE_WAIT);
+    if (RT_SUCCESS(rc))
+    {    
+        /** @todo Use RTMemCache or RTMemObj here? */
+        uint8_t *pNewBuf;
+        while (pBuf->cbSize - pBuf->cbOffset < cbData)
+        {
+            pNewBuf = (uint8_t*)RTMemRealloc(pBuf->pbData, pBuf->cbSize + _4K);
+            if (pNewBuf == NULL)
+                break;
+            pBuf->cbSize += _4K;
+            pBuf->pbData = pNewBuf;
+        }
+        
+        int rc = VINF_SUCCESS;
+        if (pBuf->pbData)
+        {
+            memcpy(pBuf->pbData + pBuf->cbOffset, pbData, cbData);
+            pBuf->cbOffset += cbData;
+            /** @todo Add offset clamping! */
+        }
+        else
+            rc = VERR_NO_MEMORY;
+        int rc2 = RTSemMutexRelease(pBuf->mtx);
+        if (RT_SUCCESS(rc))
+            rc = rc2;
+    }
     return rc;
 }
@@ -668 +704 @@
 
         /* Init buffers. */
-        pData->stdOut.pbData = NULL;
-        pData->stdOut.cbSize = 0;
-        pData->stdOut.cbOffset = 0;
-        pData->stdOut.cbRead = 0;
-
-        pData->stdErr.pbData = NULL;
-        pData->stdErr.cbSize = 0;
-        pData->stdErr.cbOffset = 0;
-        pData->stdErr.cbRead = 0;
+        rc = VBoxServiceControlExecInitPipeBuffer(&pData->stdOut);
+        if (RT_SUCCESS(rc))
+            rc = VBoxServiceControlExecInitPipeBuffer(&pData->stdErr);
     }
 
@@ -689 +719 @@
     }
     return rc;
-}
-
-void VBoxServiceControlExecDestroyPipeBuffer(PVBOXSERVICECTRLEXECPIPEBUF pBuf)
-{
-    if (pBuf)
-    {
-        if (pBuf->pbData)
-            RTMemFree(pBuf->pbData);
-        pBuf->pbData = NULL;
-        pBuf->cbSize = 0;
-        pBuf->cbOffset = 0;
-        pBuf->cbRead = 0;
-    }
 }
 

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceInternal.h

@@ -27 +27 @@
 #ifdef VBOX_WITH_GUEST_CONTROL
 # include <iprt/list.h>
+# include <iprt/semaphore.h>
 #endif
 
@@ -134 +135 @@
 typedef struct
 {
-    uint8_t  *pbData;
-    uint32_t  cbSize;
-    uint32_t  cbOffset;
-    uint32_t  cbRead;
+    uint8_t    *pbData;
+    uint32_t    cbSize;
+    uint32_t    cbOffset;
+    uint32_t    cbRead;
+    RTSEMMUTEX  mtx;
 } VBOXSERVICECTRLEXECPIPEBUF;
 /** Pointer to thread data. */

trunk/src/VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp

@@ -344 +344 @@
                 while (SUCCEEDED(progress->COMGETTER(Completed(&fCompleted))))
                 {
+                    /* 
+                     * because we want to get all the output data even if the process
+                     * already ended, we first need to check whether there is some data
+                     * left to output before checking the actual timeout and is-process-completed
+                     * stuff. 
+                     */
                     if (cbOutputData <= 0)
                     {
@@ -382 +388 @@
                     }
 
-                    /* process async cancelation. */
+                    /* process async cancelation */
                     if (g_fExecCanceled && !fCanceledAlready)
                     {
@@ -404 +410 @@
                 }
 
-                /* Not completed yet? -> Timeout */
-                if (fCompleted)
-                {
-                    LONG iRc = false;
-                    CHECK_ERROR_RET(progress, COMGETTER(ResultCode)(&iRc), rc);
-                    if (FAILED(iRc))
-                    {
-                        ComPtr<IVirtualBoxErrorInfo> execError;
-                        rc = progress->COMGETTER(ErrorInfo)(execError.asOutParam());
-                        com::ErrorInfo info (execError);
-                        RTPrintf("\n\nProcess error details:\n");
-                        GluePrintErrorInfo(info);
-                        RTPrintf("\n");
-                    }         
+                BOOL fCanceled;
+                CHECK_ERROR_RET(progress, COMGETTER(Canceled)(&fCanceled), rc);
+                if (fCanceled)
+                {
+                    RTPrintf("Process execution canceled!\n");
                 }
                 else
-                {
-                    RTPrintf("Process timed out!\n");
-                }
-                
-                if (verbose)
-                {
-                    ULONG uRetStatus, uRetExitCode, uRetFlags;
-                    CHECK_ERROR_BREAK(guest, GetProcessStatus(uPID, &uRetExitCode, &uRetFlags, &uRetStatus));
-                    RTPrintf("Exit code=%u (Status=%u, Flags=%u)\n", uRetExitCode, uRetStatus, uRetFlags);
+                {                   
+                    if (fCompleted)
+                    {
+                        LONG iRc = false;
+                        CHECK_ERROR_RET(progress, COMGETTER(ResultCode)(&iRc), rc);
+                        if (FAILED(iRc))
+                        {
+                            ComPtr<IVirtualBoxErrorInfo> execError;
+                            rc = progress->COMGETTER(ErrorInfo)(execError.asOutParam());
+                            com::ErrorInfo info (execError);
+                            RTPrintf("\n\nProcess error details:\n");
+                            GluePrintErrorInfo(info);
+                            RTPrintf("\n");
+                        }
+                        if (verbose)
+                        {
+                            ULONG uRetStatus, uRetExitCode, uRetFlags;
+                            CHECK_ERROR_BREAK(guest, GetProcessStatus(uPID, &uRetExitCode, &uRetFlags, &uRetStatus));
+                            RTPrintf("Exit code=%u (Status=%u, Flags=%u)\n", uRetExitCode, uRetStatus, uRetFlags);
+                        }       
+                    }
+                    else /* not completed yet? -> timeout */
+                    {
+                        RTPrintf("Process timed out!\n");
+                    }
                 }
             }

trunk/src/VBox/Main/GuestImpl.cpp

@@ -462 +462 @@
     int rc = VINF_SUCCESS;
 
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
     AssertPtr(pData);
     CallbackListIter it = getCtrlCallbackContextByID(pData->hdr.u32ContextID);
@@ -470 +468 @@
     if (it != mCallbackList.end())
     {
+        AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
         PHOSTEXECCALLBACKDATA pCBData = (HOSTEXECCALLBACKDATA*)it->pvData;
         AssertPtr(pCBData);
@@ -655 +655 @@
             && !it->pProgress->getCompleted())
         {
-            it->pProgress->notifyComplete(S_OK);
+            /* Only notify as complete if not canceled! */
+            BOOL fCancelled;
+            if (SUCCEEDED(it->pProgress->COMGETTER(Canceled)(&fCancelled)) && !fCancelled)
+                it->pProgress->notifyComplete(S_OK);
             it->pProgress = NULL;
         }