Changeset 23827 in vbox for trunk/src

Timestamp:

Oct 16, 2009 3:50:35 PM (16 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

53583

Message:

Main: Cancellation fixes (almost perfect).

Location:

trunk/src/VBox/Main

Files:

• ConsoleImpl.cpp (modified) (1 diff)
• ConsoleImplTeleporter.cpp (modified) (14 diffs)
• ProgressImpl.cpp (modified) (3 diffs)
• include/ConsoleImpl.h (modified) (1 diff)
• include/ProgressImpl.h (modified) (2 diffs)

trunk/src/VBox/Main/ConsoleImpl.cpp

@@ -6762 +6762 @@
                 }
                 else if (task->mTeleporterEnabled)
-                {
                     /* -> ConsoleImplTeleporter.cpp */
                     vrc = console->teleporterTrg(pVM, pMachine, task->mStartPaused, task->mProgress);
-                    if (RT_FAILURE(vrc))
-                        VMR3PowerOff(pVM);
-                }
                 else if (task->mStartPaused)
                     /* done */

trunk/src/VBox/Main/ConsoleImplTeleporter.cpp

@@ -240 +240 @@
  * @param   pState              The teleporter source state.
  * @param   pszCommand          The command.
+ * @param   fWaitForAck         Whether to wait for the ACK.
  *
  * @remarks the setError laziness forces this to be a Console member.
  */
 HRESULT
-Console::teleporterSrcSubmitCommand(TeleporterStateSrc *pState, const char *pszCommand)
+Console::teleporterSrcSubmitCommand(TeleporterStateSrc *pState, const char *pszCommand, bool fWaitForAck /*= true*/)
 {
     size_t cchCommand = strlen(pszCommand);
@@ -254 +255 @@
     if (RT_FAILURE(vrc))
         return setError(E_FAIL, tr("Failed writing command '%s': %Rrc"), pszCommand, vrc);
+    if (!fWaitForAck)
+        return S_OK;
     return teleporterSrcReadACK(pState, pszCommand);
 }
@@ -521 +524 @@
         {
             /* check if the failure was caused by cancellation. */
-            BOOL fCanceled;
-            hrc = pState->mptrProgress->COMGETTER(Canceled)(&fCanceled);
-            if (SUCCEEDED(hrc) && fCanceled)
+            BOOL fCancelled;
+            hrc = pState->mptrProgress->COMGETTER(Canceled)(&fCancelled);
+            if (SUCCEEDED(hrc) && fCancelled)
             {
                 SSMR3Cancel(pState->mpVM);
@@ -562 +565 @@
     { AutoWriteLock autoLock(); }
 
-    BOOL fCanceled = TRUE;
-    HRESULT hrc = pState->mptrProgress->COMGETTER(Canceled)(&fCanceled);
+    BOOL fCancelled = TRUE;
+    HRESULT hrc = pState->mptrProgress->COMGETTER(Canceled)(&fCancelled);
     if (FAILED(hrc))
         return hrc;
-    if (fCanceled)
-        return setError(E_FAIL, tr("canceled"));
+    if (fCancelled)
+        return setError(E_FAIL, tr("cancelled"));
 
     /*
@@ -624 +627 @@
      * State fun? Automatic power off?
      */
+    if (!pState->mptrProgress->notifyPointOfNoReturn())
+    {
+        teleporterSrcSubmitCommand(pState, "cancel", false /*fWaitForAck*/);
+        return E_FAIL;
+    }
     hrc = teleporterSrcSubmitCommand(pState, "done");
     if (FAILED(hrc))
@@ -650 +658 @@
         hrc = pState->mptrConsole->teleporterSrc(pState);
 
+    /* (Ignore the return here as teleporterSrc deals with cancellation.) */
     pState->mptrProgress->notifyComplete(hrc);
-
+    pState->mptrProgress->setCancelCallback(NULL, NULL);
+
+    /*
+     * Deal with the state machinery after taking the console object lock.
+     */
     AutoWriteLock autoLock(pState->mptrConsole);
     if (pState->mptrConsole->mMachineState == MachineState_Saving)
@@ -695 +708 @@
     }
 
+    /*
+     * Cleanup.
+     */
     if (pState->mhSocket != NIL_RTSOCKET)
     {
@@ -700 +716 @@
         pState->mhSocket = NIL_RTSOCKET;
     }
-
-    pState->mptrProgress->setCancelCallback(NULL, NULL);
     delete pState;
 
-    return VINF_SUCCESS;
+    return VINF_SUCCESS; /* ignored */
 }
 
@@ -847 +861 @@
         if (RT_SUCCESS(vrc))
         {
-            HRESULT hrc = pMachine->COMSETTER(TeleporterPort)(uPort);
+            hrc = pMachine->COMSETTER(TeleporterPort)(uPort);
             if (FAILED(hrc))
             {
@@ -876 +890 @@
 
             void *pvUser = static_cast<void *>(static_cast<TeleporterState *>(&State));
-            pProgress->setCancelCallback(teleporterProgressCancelCallback, pvUser);
-
-            vrc = RTTcpServerListen(hServer, Console::teleporterTrgServeConnection, &State);
-            if (vrc == VERR_TCP_SERVER_STOP)
-                vrc = State.mRc;
-            if (RT_SUCCESS(vrc))
+            if (pProgress->setCancelCallback(teleporterProgressCancelCallback, pvUser))
             {
-                if (fStartPaused)
-                    setMachineState(MachineState_Paused);
+                vrc = RTTcpServerListen(hServer, Console::teleporterTrgServeConnection, &State);
+                pProgress->setCancelCallback(NULL, NULL);
+
+                bool fPowerOff = false;
+                if (vrc == VERR_TCP_SERVER_STOP)
+                {
+                    vrc = State.mRc;
+                    if (RT_SUCCESS(vrc))
+                    {
+                        if (fStartPaused)
+                            setMachineState(MachineState_Paused);
+                        else
+                            vrc = VMR3Resume(pVM);
+                    }
+                    /* Power off the VM on failure unless the state callback
+                       already did that. */
+                    else
+                    {
+                        VMSTATE enmVMState = VMR3GetState(pVM);
+                        if (    enmVMState != VMSTATE_OFF
+                            &&  enmVMState != VMSTATE_POWERING_OFF)
+                            fPowerOff = true;
+                    }
+                }
+                else if (vrc == VERR_TCP_SERVER_SHUTDOWN)
+                {
+                    /** @todo this crap isn't work right wrt error info. Aaaarrrg! */
+                    BOOL fCancelled = TRUE;
+                    hrc = pProgress->COMGETTER(Canceled)(&fCancelled);
+                    if (FAILED(hrc) || fCancelled)
+                    {
+                        setError(E_FAIL, tr("Teleporting canceled"));
+                        vrc = VERR_SSM_CANCELLED;
+                    }
+                    else
+                    {
+                        setError(E_FAIL, tr("Teleporter timed out waiting for incoming connection"));
+                        vrc = VERR_TIMEOUT;
+                    }
+                    pProgress->setResultCode(E_FAIL); /* ugly! */
+                    LogRel(("Teleporter: RTTcpServerListen aborted - %Rrc\n", vrc));
+                    fPowerOff = true;
+                }
                 else
-                    vrc = VMR3Resume(pVM);
+                {
+                    LogRel(("Teleporter: Unexpected RTTcpServerListen rc: %Rrc\n", vrc));
+                    vrc = VERR_IPE_UNEXPECTED_STATUS;
+                    fPowerOff = true;
+                }
+
+                if (fPowerOff)
+                {
+                    int vrc2 = VMR3PowerOff(pVM);
+                    AssertRC(vrc2);
+                }
             }
             else
-            {
-                LogRel(("Teleporter: RTTcpServerListen -> %Rrc\n", vrc));
-            }
-
-            pProgress->setCancelCallback(NULL, NULL);
+                vrc = VERR_SSM_CANCELLED;
         }
 
@@ -935 +991 @@
 /**
  * @copydoc FNRTTCPSERVE
+ *
+ * @returns VINF_SUCCESS or VERR_TCP_SERVER_STOP.
  */
 /*static*/ DECLCALLBACK(int)
@@ -953 +1011 @@
 
     /*
-     * Password (includes '\n', see teleporterTrg).  If it's right, tell
-     * the TCP server to stop listening (frees up host resources and makes sure
-     * this is the last connection attempt).
+     * Password (includes '\n', see teleporterTrg).
      */
     const char *pszPassword = pState->mstrPassword.c_str();
@@ -977 +1033 @@
     vrc = teleporterTcpWriteACK(pState);
     if (RT_FAILURE(vrc))
-        return vrc;
-
+        return VINF_SUCCESS;
+
+    /*
+     * Stop the server and cancel the timeout timer.
+     *
+     * Note! After this point we must return VERR_TCP_SERVER_STOP, while prior
+     *       to it we must not return that value!
+     */
     RTTcpServerShutdown(pState->mhServer);
     RTTimerLRDestroy(*pState->mphTimerLR);
@@ -1028 +1090 @@
          *        maybe leave part of these to the saved state machinery?
          * Update: We're doing as much as possible in the first SSM pass. */
+        else if (!strcmp(szCmd, "cancel"))
+        {
+            /* Don't ACK this. */
+            LogRel(("Teleporter: Received cancel command.\n"));
+            vrc = VERR_SSM_CANCELLED;
+        }
         else if (!strcmp(szCmd, "done"))
         {
-            vrc = teleporterTcpWriteACK(pState);
+            /*
+             * The ACK is the point of no return.
+             */
+            if (pState->mptrProgress->notifyPointOfNoReturn())
+                vrc = teleporterTcpWriteACK(pState);
+            else
+            {
+                vrc = VERR_SSM_CANCELLED;
+                teleporterTcpWriteNACK(pState, vrc);
+            }
             break;
         }

trunk/src/VBox/Main/ProgressImpl.cpp

@@ -514 +514 @@
 
 /**
- * Sets the cancellation callback.
+ * Sets the cancelation callback, checking for cancelation first.
+ *
+ * @returns Success indicator.
+ * @retval  true on success.
+ * @retval  false if the progress object has already been canceled or is in an
+ *          invalid state
  *
  * @param   pfnCallback     The function to be called upon cancelation.
  * @param   pvUser          The callback argument.
  */
-void ProgressBase::setCancelCallback(void (*pfnCallback)(void *), void *pvUser)
-{
-    AutoCaller autoCaller(this);
-    AssertComRCReturnVoid(autoCaller.rc());
-
-    AutoWriteLock alock(this);
+bool ProgressBase::setCancelCallback(void (*pfnCallback)(void *), void *pvUser)
+{
+    AutoCaller autoCaller(this);
+    AssertComRCReturn(autoCaller.rc(), false);
+
+    AutoWriteLock alock(this);
+
+    if (mCanceled)
+        return false;
 
     m_pvCancelUserArg   = pvUser;
     m_pfnCancelCallback = pfnCallback;
-}
-
+    return true;
+}
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1141 +1149 @@
 }
 
+/**
+ * Notify the progress object that we're almost at the point of no return.
+ *
+ * This atomically checks for and disables cancelation.  Calls to
+ * IProgress::Cancel() made after a successfull call to this method will fail
+ * and the user can be told.  While this isn't entirely clean behavior, it
+ * prevents issues with an irreversible actually operation succeeding while the
+ * user belive it was rolled back.
+ *
+ * @returns Success indicator.
+ * @retval  true on success.
+ * @retval  false if the progress object has already been canceled or is in an
+ *          invalid state
+ */
+bool Progress::notifyPointOfNoReturn(void)
+{
+    AutoCaller autoCaller(this);
+    AssertComRCReturn(autoCaller.rc(), false);
+
+    AutoWriteLock alock(this);
+
+    if (mCanceled)
+        return false;
+
+    mCancelable = FALSE;
+    return true;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // CombinedProgress class
@@ -1591 +1627 @@
 
     if (!mCancelable)
-        return setError (E_FAIL, tr ("Operation cannot be cancelled"));
+        return setError (E_FAIL, tr ("Operation cannot be canceled"));
 
     if (!mCanceled)

trunk/src/VBox/Main/include/ConsoleImpl.h

@@ -522 +522 @@
     HRESULT                     teleporterSrc(TeleporterStateSrc *pState);
     HRESULT                     teleporterSrcReadACK(TeleporterStateSrc *pState, const char *pszWhich, const char *pszNAckMsg = NULL);
-    HRESULT                     teleporterSrcSubmitCommand(TeleporterStateSrc *pState, const char *pszCommand);
+    HRESULT                     teleporterSrcSubmitCommand(TeleporterStateSrc *pState, const char *pszCommand, bool fWaitForAck = true);
     int                         teleporterTrg(PVM pVM, IMachine *pMachine, bool fStartPaused, Progress *pProgress);
     static DECLCALLBACK(int)    teleporterTrgServeConnection(RTSOCKET Sock, void *pvUser);

trunk/src/VBox/Main/include/ProgressImpl.h

@@ -86 +86 @@
 
     static HRESULT setErrorInfoOnThread (IProgress *aProgress);
-    void setCancelCallback(void (*pfnCallback)(void *), void *pvUser);
+    bool setCancelCallback(void (*pfnCallback)(void *), void *pvUser);
 
 
@@ -266 +266 @@
                            const Bstr &aComponent,
                            const char *aText, ...);
+    bool notifyPointOfNoReturn(void);
 
     /** For com::SupportErrorInfoImpl. */