Changeset 42123 in vbox for trunk/src/VBox/Main/src-server

Timestamp:

Jul 12, 2012 10:10:36 AM (13 years ago)

Author:

vboxsync

Message:

Main/Machine+Snapshot: fix crash on snapshot restore due to incorrect saving of settings (#6281)

Location:

trunk/src/VBox/Main/src-server

Files:

• MachineImpl.cpp (modified) (2 diffs)
• SnapshotImpl.cpp (modified) (2 diffs)

trunk/src/VBox/Main/src-server/MachineImpl.cpp

@@ -13187 +13187 @@
  *
  * @note Locks this object for writing.
+ * @note This method must not call saveSettings or SaveSettings, otherwise
+ *       it can cause crashes in random places due to unexpectedly committing
+ *       the current settings. The caller is responsible for that. The call
+ *       to saveStateSettings is fine, because this method does not commit.
  */
 HRESULT SessionMachine::setMachineState(MachineState_T aMachineState)
@@ -13385 +13389 @@
             mData->mCurrentStateModified = TRUE;
             stsFlags |= SaveSTS_CurStateModified;
-            SaveSettings();     // @todo r=dj why the public method? why first SaveSettings and then saveStateSettings?
         }
     }

trunk/src/VBox/Main/src-server/SnapshotImpl.cpp

@@ -1905 +1905 @@
         stateRestored = true;
 
+        /* Paranoia: no one must have saved the settings in the mean time. If
+         * it happens nevertheless we'll close our eyes and continue below. */
+        Assert(mMediaData.isBackedUp());
+
         /* assign the timestamp from the snapshot */
-        Assert(RTTimeSpecGetMilli (&snapshotTimeStamp) != 0);
+        Assert(RTTimeSpecGetMilli(&snapshotTimeStamp) != 0);
         mData->mLastStateChange = snapshotTimeStamp;
 
@@ -1932 +1936 @@
             // remove from machine data
             mMediaData->mAttachments.remove(pAttach);
-            // remove it from the backup or else saveSettings will try to detach
-            // it again and assert
-            mMediaData.backedUpData()->mAttachments.remove(pAttach);
+            // Remove it from the backup or else saveSettings will try to detach
+            // it again and assert. The paranoia check avoids crashes (see
+            // assert above) if this code is buggy and saves settings in the
+            // wrong place.
+            if (mMediaData.isBackedUp())
+                mMediaData.backedUpData()->mAttachments.remove(pAttach);
             // then clean up backrefs
             pMedium->removeBackReference(mData->mUuid);