Changeset 2804 in vbox for trunk/src

Timestamp:

May 23, 2007 1:44:21 PM (18 years ago)

Author:

vboxsync

Message:

Main: Added Machine::AutoStateDependency smart class that helps to ensure the machine state won't unexpectedly change, in a lock-free manner.

Location:

trunk/src/VBox/Main

Files:

• MachineImpl.cpp (modified) (15 diffs)
• include/MachineImpl.h (modified) (8 diffs)

trunk/src/VBox/Main/MachineImpl.cpp

@@ -124 +124 @@
     RTTIMESPEC time;
     mLastStateChange = RTTimeSpecGetMilli (RTTimeNow (&time));
+
+    mMachineStateDeps = 0;
+    mZeroMachineStateDepsSem = NIL_RTSEMEVENT;
+    mWaitingStateDeps = FALSE;
+
     mCurrentStateModified = TRUE;
     mHandleCfgFile = NIL_RTFILE;
@@ -133 +138 @@
 Machine::Data::~Data()
 {
+    if (mZeroMachineStateDepsSem != NIL_RTSEMEVENT)
+    {
+        RTSemEventDestroy (mZeroMachineStateDepsSem);
+        mZeroMachineStateDepsSem = NIL_RTSEMEVENT;
+    }
 }
 
@@ -2917 +2927 @@
     AutoLock alock (this);
 
+    /* wait for state dependants to drop to zero */
+    checkStateDependencies (alock);
+
     ComAssertRet (mData->mRegistered != aRegistered, E_FAIL);
 
@@ -2996 +3009 @@
 }
 
+/** 
+ *  Increases the number of objects dependent on the machine state or on the
+ *  registered state.  Guarantees that these two states will not change at
+ *  least until #releaseStateDependency() is called.
+ *
+ *  Depending on the @a aDepType value, additional state checks may be
+ *  made. These checks will set extended error info on failure.
+ * 
+ *  If this method returns a failure, the dependency is not added and the
+ *  caller is not allowed to rely on any particular machine state or
+ *  registration state value and may return the failed result code to the
+ *  upper level.
+ *
+ *  @param aDepType     Dependency type to choose
+ *  @param aState       Current machine state (NULL if not interested).
+ *  @param aRegistered  Current registered state (NULL if not interested).
+ */
+HRESULT Machine::addStateDependency (StateDependency aDepType /* = AnyStateDep */,
+                                     MachineState_T *aState /* = NULL */,
+                                     BOOL *aRegistered /* = NULL */)
+{
+    AutoCaller autoCaller (this);
+    AssertComRCReturnRC (autoCaller.rc());
+
+    AutoLock alock (this);
+
+    if (mData->mWaitingStateDeps && mData->mMachineStateDeps == 0)
+    {
+        /* checkStateDependencies() is at the point after RTSemEventWait() but
+         * before entering the lock. Report an error. It would be better to
+         * leave the lock now and re-schedule ourselves, but we don't have a
+         * framework that can guarantee such a behavior in 100% cases. */
+
+        AssertFailed(); /* <-- this is just to see how often it can happen */
+
+        return setError (E_ACCESSDENIED,
+            tr ("The machine is busy: state transition is in progress. "
+                "Retry the operation (state is %d)"),
+            mData->mMachineState);
+    }
+
+    switch (aDepType)
+    {
+        case AnyStateDep:
+        {
+            break;
+        }
+        case MutableStateDep:
+        {
+            if (mData->mRegistered &&
+                (mType != IsSessionMachine ||
+                 mData->mMachineState > MachineState_Paused ||
+                 mData->mMachineState == MachineState_Saved))
+                return setError (E_ACCESSDENIED,
+                    tr ("The machine is not mutable (state is %d)"),
+                    mData->mMachineState);
+            break;
+        }
+        case MutableOrSavedStateDep:
+        {
+            if (mData->mRegistered &&
+                (mType != IsSessionMachine ||
+                 mData->mMachineState > MachineState_Paused))
+                return setError (E_ACCESSDENIED,
+                    tr ("The machine is not mutable (state is %d)"),
+                    mData->mMachineState);
+            break;
+        }
+    }
+
+    if (aState)
+        *aState = mData->mMachineState;
+    if (aRegistered)
+        *aRegistered = mData->mRegistered;
+
+    ++ mData->mMachineStateDeps;
+
+    return S_OK;
+}
+
+/**
+ *  Decreases the number of objects dependent on the machine state.
+ *  Must always complete the #addStateDependency() call after the state
+ *  dependency no more necessary.
+ */
+void Machine::releaseStateDependency()
+{
+    AutoCaller autoCaller (this);
+    AssertComRCReturnVoid (autoCaller.rc());
+
+    AutoLock alock (this);
+
+    AssertReturnVoid (mData->mMachineStateDeps > 0);
+    -- mData->mMachineStateDeps;
+
+    if (mData->mMachineStateDeps == 0 &&
+        mData->mZeroMachineStateDepsSem != NIL_RTSEMEVENT)
+    {
+        /* inform checkStateDependencies() that there are no more deps */
+        RTSemEventSignal (mData->mZeroMachineStateDepsSem);
+    }
+}
+
 // protected methods
 /////////////////////////////////////////////////////////////////////////////
@@ -3077 +3193 @@
 }
 
+
+/** 
+ *  Chhecks that there are no state dependants. If necessary, waits for the
+ *  number of dependants to drop to zero. Must be called from under
+ *  this object's lock.
+ *
+ *  @param aLock    This object's lock.
+ *
+ *  @note This method may leave the object lock during its execution!
+ */
+void Machine::checkStateDependencies (AutoLock &aLock)
+{
+    AssertReturnVoid (isLockedOnCurrentThread());
+    AssertReturnVoid (aLock.belongsTo (this));
+
+    /* Wait for all state dependants if necessary */
+    if (mData->mMachineStateDeps > 0)
+    {
+        /* lazy creation */
+        if (mData->mZeroMachineStateDepsSem == NIL_RTSEMEVENT)
+            RTSemEventCreate (&mData->mZeroMachineStateDepsSem);
+
+        LogFlowThisFunc (("Waiting for state deps (%d) to drop to zero...\n",
+                          mData->mMachineStateDeps));
+
+        mData->mWaitingStateDeps = TRUE;
+
+        aLock.leave();
+
+        RTSemEventWait (mData->mZeroMachineStateDepsSem, RT_INDEFINITE_WAIT);
+
+        aLock.enter();
+
+        mData->mWaitingStateDeps = FALSE;
+    }
+}
+
 /**
  *  Helper to change the machine state.
@@ -3091 +3244 @@
 
     AutoLock alock (this);
+
+    /* wait for state dependants to drop to zero */
+        /// @todo it may be potentially unsafe to leave the lock here as
+    //  the below method does. Needs some thinking. The easiest solution may
+    //  be to provide a separate mutex for mMachineState and mRegistered.
+    checkStateDependencies (alock);
 
     if (mData->mMachineState != aMachineState)
@@ -3377 +3536 @@
             mSSData->mStateFilePath.setNull();
 
+            /* no need to use setMachineState() during init() */ 
             mData->mMachineState = MachineState_Aborted;
         }
         else if (mSSData->mStateFilePath)
         {
+            /* no need to use setMachineState() during init() */ 
             mData->mMachineState = MachineState_Saved;
         }
@@ -7291 +7452 @@
          *  below, the following is enough.
          */
-        LogFlowThisFunc (("Initialization failed\n"));
+        LogFlowThisFunc (("Initialization failed.\n"));
 #if defined(__WIN__)
         if (mIPCSem)
@@ -7315 +7476 @@
     AutoMultiLock <2> alock (mParent->wlock(), this->wlock());
 
+    MachineState_T lastState = mData->mMachineState;
+
+    if (aReason == Uninit::Abnormal)
+    {
+        LogWarningThisFunc (("ABNORMAL client termination! (wasRunning=%d)\n",
+                             lastState >= MachineState_Running));
+
+        /* reset the state to Aborted */
+        if (mData->mMachineState != MachineState_Aborted)
+            setMachineState (MachineState_Aborted);
+    }
+
     if (isModified())
     {
@@ -7334 +7507 @@
 
     /* release all captured USB devices */
-    mParent->host()->releaseAllUSBDevices (this);
+    if (aReason == Uninit::Abnormal && lastState >= MachineState_Running)
+    {
+        /* Console::captureUSBDevices() is called in the VM process only after
+         * setting the machine state to Starting or Restoring.
+         * Console::releaseAllUSBDevices() will be called upon successful
+         * termination. So, we need to release USB devices only if there was
+         * an abnormal termination of a running VM. */
+        ReleaseAllUSBDevices();
+    }
 
     if (!mData->mSession.mType.isNull())
@@ -7557 +7738 @@
 
 /**
- *  @note Locks the same as Host::autoCaptureUSBDevices() does.
+ *  Inserts all machine filters to the USB proxy service and then calls
+ *  Host::autoCaptureUSBDevices().
+ *
+ *  Called by Console from the VM process upon VM startup.
+ *
+ *  @note Locks what called methods lock.
  */
 STDMETHODIMP SessionMachine::AutoCaptureUSBDevices (IUSBDeviceCollection **aHostDevices)
@@ -7566 +7752 @@
     AssertComRCReturn (autoCaller.rc(), autoCaller.rc());
 
+    HRESULT rc = mUSBController->notifyProxy (true /* aInsertFilters */);
+    AssertComRC (rc);
+    NOREF (rc);
+
     return mParent->host()->autoCaptureUSBDevices (this, aHostDevices);
 }
 
 /**
- *  @note Locks the same as Host::releaseAllUSBDevices() does.
+ *  Removes all machine filters from the USB proxy service and then calls
+ *  Host::releaseAllUSBDevices().
+ *
+ *  Called by Console from the VM process upon normal VM termination or by
+ *  SessionMachine::uninit() upon abnormal VM termination (from under the
+ *  Machine/SessionMachine lock).
+ *
+ *  @note Locks what called methods lock.
  */
 STDMETHODIMP SessionMachine::ReleaseAllUSBDevices()
@@ -7578 +7775 @@
     AutoCaller autoCaller (this);
     AssertComRCReturn (autoCaller.rc(), autoCaller.rc());
+
+    HRESULT rc = mUSBController->notifyProxy (false /* aInsertFilters */);
+    AssertComRC (rc);
+    NOREF (rc);
 
     return mParent->host()->releaseAllUSBDevices (this);
@@ -8235 +8436 @@
         AssertMsg (mIPCSem, ("semaphore must be created"));
 
-        if (reason == Uninit::Abnormal)
-        {
-            LogWarningThisFunc (("ABNORMAL client termination! (wasRunning=%d)\n",
-                                 mData->mMachineState >= MachineState_Running));
-
-            /* reset the state to Aborted */
-            if (mData->mMachineState != MachineState_Aborted)
-                setMachineState (MachineState_Aborted);
-        }
-
         /* release the IPC mutex */
         ::ReleaseMutex (mIPCSem);
@@ -8260 +8451 @@
         {
             /* the semaphore is signaled, meaning the session is terminated */
-
-            if (reason == Uninit::Abnormal)
-            {
-                LogWarningThisFunc (("ABNORMAL client termination! (wasRunning=%d)\n",
-                                     mData->mMachineState >= MachineState_Running));
-
-                /* reset the state to Aborted */
-                if (mData->mMachineState != MachineState_Aborted)
-                    setMachineState (MachineState_Aborted);
-            }
-
             doUninit = true;
         }

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

@@ -59 +59 @@
  *  @param machine  the machine object (must cast to Machine *)
  */
+/// @todo replace with AutoStateDependency
 #define CHECK_MACHINE_MUTABILITY(machine) \
     do { \
@@ -64 +65 @@
             return setError (E_ACCESSDENIED, tr ("The machine is not mutable")); \
     } while (0)
-/** like CHECK_MACHINE_MUTABILITY but a saved state is ok, too */
+
+/** Like CHECK_MACHINE_MUTABILITY but a saved state is OK, too. */
+/// @todo replace with AutoStateDependency
 #define CHECK_MACHINE_MUTABILITY_IGNORING_SAVED(machine) \
     do { \
         if (!machine->isMutableIgnoringSavedState()) \
-            return setError (E_ACCESSDENIED, tr ("The machine is not mutable or in saved state")); \
+            return setError (E_ACCESSDENIED, \
+                             tr ("The machine is not mutable or in saved state")); \
     } while (0)
 
@@ -167 +171 @@
         LONG64 mLastStateChange;
 
+        uint32_t mMachineStateDeps;
+        RTSEMEVENT mZeroMachineStateDepsSem;
+        BOOL mWaitingStateDeps;
+
         BOOL mCurrentStateModified;
 
@@ -277 +285 @@
          */
         bool mHDAttachmentsChanged;
+    };
+
+    enum StateDependency
+    {
+        AnyStateDep = 0, MutableStateDep, MutableOrSavedStateDep
+    };
+
+    /**
+     *  Helper class that safely manages the machine state dependency by
+     *  calling Machine::addStateDependency() on construction and
+     *  Machine::releaseStateDependency() on destruction. Intended for Machine
+     *  children. The usage pattern is:
+     *
+     *  @code
+     *      Machine::AutoStateDependency <MutableStateDep> adep (mParent);
+     *      CheckComRCReturnRC (stateDep.rc());
+     *      ...
+     *      // code that depends on the particular machine state
+     *      ...
+     *  @endcode
+     *
+     *  @param taDepType    Dependecy type to manage.
+     */
+    template <StateDependency taDepType = AnyStateDep>
+    class AutoStateDependency
+    {
+    public:
+
+        AutoStateDependency (Machine *aThat)
+            : mThat (aThat), mRC (S_OK)
+            , mMachineState (MachineState_InvalidMachineState)
+            , mRegistered (FALSE)
+        {
+            Assert (aThat);
+            mRC = aThat->addStateDependency (taDepType, &mMachineState,
+                                             &mRegistered);
+        }
+        ~AutoStateDependency()
+        {
+            if (SUCCEEDED (mRC))
+                mThat->releaseStateDependency();
+        }
+
+        /** Decreases the number of dependencies before the instance is
+         *  destroyed. Note that will reset #rc() to E_FAIL. */
+        void release()
+        {
+            AssertReturnVoid (SUCCEEDED (mRC));
+            mThat->releaseStateDependency();
+            mRC = E_FAIL;
+        }
+
+        /** Restores the number of callers after by #release(). #rc() will be
+         *  reset to the result of calling addStateDependency() and must be
+         *  rechecked to ensure the operation succeeded. */
+        void add()
+        {
+            AssertReturnVoid (!SUCCEEDED (mRC));
+            mRC = mThat->addStateDependency (taDepType, &mMachineState,
+                                             &mRegistered);
+        }
+
+        /** Returns the result of Machine::addStateDependency(). */
+        HRESULT rc() const { return mRC; }
+
+        /** Shortcut to SUCCEEDED (rc()). */
+        bool isOk() const { return SUCCEEDED (mRC); }
+
+        /** Returns the machine state value as returned by
+         *  Machine::addStateDependency(). */
+        MachineState_T machineState() const { return mMachineState; }
+
+        /** Returns the machine state value as returned by
+         *  Machine::addStateDependency(). */
+        BOOL machineRegistered() const { return mRegistered; }
+
+    protected:
+
+        Machine *mThat;
+        HRESULT mRC;
+        MachineState_T mMachineState;
+        BOOL mRegistered;
+
+    private:
+
+        DECLARE_CLS_COPY_CTOR_ASSIGN_NOOP (AutoStateDependency)
+        DECLARE_CLS_NEW_DELETE_NOOP (AutoStateDependency)
     };
 
@@ -375 +470 @@
     //  Note: these classes should enter Machine lock to keep the returned
     //  information valid!
+    /// @todo replace with AutoStateDependency
     bool isMutable()
     {
@@ -387 +483 @@
     //  Note: these classes should enter Machine lock to keep the returned
     //  information valid!
+    /// @todo replace with AutoStateDependency
     bool isMutableIgnoringSavedState()
     {
@@ -446 +543 @@
     }
 
+    HRESULT addStateDependency (StateDependency aDepType = AnyStateDep,
+                                MachineState_T *aState = NULL,
+                                BOOL *aRegistered = NULL);
+    void releaseStateDependency();
+
     // for VirtualBoxSupportErrorInfoImpl
     static const wchar_t *getComponentName() { return L"Machine"; }
@@ -458 +560 @@
 
     void uninitDataAndChildObjects();
+
+    void checkStateDependencies (AutoLock &aLock);
 
     virtual HRESULT setMachineState (MachineState_T aMachineState);