Changeset 78072 in vbox

Timestamp:

Apr 10, 2019 10:04:06 AM (6 years ago)

Author:

vboxsync

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

129928

Message:

Recording/Main: Settings locking fixes.

Location:

trunk/src/VBox/Main/src-server

Files:

• MachineImpl.cpp (modified) (1 diff)
• RecordingScreenSettingsImpl.cpp (modified) (2 diffs)
• RecordingSettingsImpl.cpp (modified) (9 diffs)

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

@@ -1947 +1947 @@
 HRESULT Machine::getRecordingSettings(ComPtr<IRecordingSettings> &aRecordingSettings)
 {
-    /* mRecordingSettings is constant during life time, no need to lock */
+    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+
     aRecordingSettings = mRecordingSettings;
 

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

@@ -140 +140 @@
     unconst(m->pPeer)   = aThat;
 
-    AutoWriteLock thatlock(aThat COMMA_LOCKVAL_SRC_POS);
+    AutoCaller thatCaller(aThat);
+    AssertComRCReturnRC(thatCaller.rc());
+
+    AutoReadLock thatlock(aThat COMMA_LOCKVAL_SRC_POS);
 
     m->uScreenId = aThat->m->uScreenId;
@@ -183 +186 @@
     /* mPeer is left null. */
 
-    AutoWriteLock thatlock(aThat COMMA_LOCKVAL_SRC_POS);
+    AutoCaller thatCaller(aThat);
+    AssertComRCReturnRC(thatCaller.rc());
+
+    AutoReadLock thatlock(aThat COMMA_LOCKVAL_SRC_POS);
 
     m->uScreenId = aThat->m->uScreenId;

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

@@ -100 +100 @@
  *  @note This object must be destroyed before the original object
  *  it shares data with is destroyed.
+ *
+ *  @note Locks @a aThat object for reading.
  */
 HRESULT RecordingSettings::init(Machine *aParent, RecordingSettings *aThat)
@@ -117 +119 @@
     unconst(m->pPeer)    = aThat;
 
-    AutoWriteLock thatlock(aThat COMMA_LOCKVAL_SRC_POS);
+    AutoCaller thatCaller(aThat);
+    AssertComRCReturnRC(thatCaller.rc());
+
+    AutoReadLock thatlock(aThat COMMA_LOCKVAL_SRC_POS);
 
     m->bd.share(aThat->m->bd);
@@ -132 +137 @@
  *  (a kind of copy constructor). This object makes a private copy of data
  *  of the original object passed as an argument.
+ *
+ *  @note Locks @a aThat object for reading.
  */
 HRESULT RecordingSettings::initCopy(Machine *aParent, RecordingSettings *aThat)
@@ -149 +156 @@
     // mPeer is left null
 
-    AutoWriteLock thatlock(aThat COMMA_LOCKVAL_SRC_POS);
+    AutoReadLock thatlock(aThat COMMA_LOCKVAL_SRC_POS);
 
     m->bd.attachCopy(aThat->m->bd);
@@ -201 +208 @@
 HRESULT RecordingSettings::setEnabled(BOOL enable)
 {
-    LogFlowThisFuncEnter();
+    /* the machine needs to be mutable */
+    AutoMutableOrSavedOrRunningStateDependency adep(m->pMachine);
+    if (FAILED(adep.rc())) return adep.rc();
 
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
@@ -215 +224 @@
 
         alock.release();
+
         rc = m->pMachine->i_onRecordingChange(enable);
         if (FAILED(rc))
@@ -232 +242 @@
             m->pMachine->i_setModified(Machine::IsModified_Recording);
 
+            /* Make sure to release the mutable dependency lock from above before
+             * actually saving the settings. */
+            adep.release();
+
             /** Save settings if online - @todo why is this required? -- @bugref{6818} */
             if (Global::IsOnline(m->pMachine->i_getMachineState()))
@@ -492 +506 @@
 void RecordingSettings::i_rollback()
 {
+    /* sanity */
+    AutoCaller autoCaller(this);
+    AssertComRCReturnVoid(autoCaller.rc());
+
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
     m->bd.rollback();
@@ -516 +534 @@
         {
             /* attach new data to the peer and reshare it */
-            AutoWriteLock peerlock(m->pPeer COMMA_LOCKVAL_SRC_POS);
             m->pPeer->m->bd.attach(m->bd);
         }