Changeset 90381 in vbox for trunk/src/VBox/VMM/VMMAll

Timestamp:

Jul 28, 2021 9:58:54 PM (4 years ago)

Author:

vboxsync

Message:

VMM/PDMCritSect: Don't sleep forever. bugref:6695

File:

• trunk/src/VBox/VMM/VMMAll/PDMAllCritSect.cpp (modified) (11 diffs)

trunk/src/VBox/VMM/VMMAll/PDMAllCritSect.cpp

@@ -129 +129 @@
  * @param   hNativeSelf         The native thread handle.
  * @param   pSrcPos             The source position of the lock operation.
- */
-static int pdmR3R0CritSectEnterContended(PVMCC pVM, PPDMCRITSECT pCritSect, RTNATIVETHREAD hNativeSelf, PCRTLOCKVALSRCPOS pSrcPos)
+ * @param   rcBusy              The status code to return when we're in RC or R0
+ */
+static int pdmR3R0CritSectEnterContended(PVMCC pVM, PPDMCRITSECT pCritSect, RTNATIVETHREAD hNativeSelf,
+                                         PCRTLOCKVALSRCPOS pSrcPos, int rcBusy)
 {
     /*
@@ -169 +171 @@
          * been signalled and the interruptible wait function returning
          * immediately.  In that case we do normal R0/RC rcBusy handling.
+         *
+         * We always do a timed wait here, so the event handle is revalidated
+         * regularly and we won't end up stuck waiting for a destroyed critsect.
          */
+        /** @todo Make SUPSemEventClose wake up all waiters. */
 # ifdef IN_RING3
 #  ifdef PDMCRITSECT_STRICT
@@ -180 +186 @@
         RTThreadBlocking(hThreadSelf, RTTHREADSTATE_CRITSECT, true);
 #  endif
-        int rc = SUPSemEventWaitNoResume(pSession, hEvent, RT_INDEFINITE_WAIT);
+        int rc = SUPSemEventWaitNoResume(pSession, hEvent, RT_MS_5SEC);
         RTThreadUnblocked(hThreadSelf, RTTHREADSTATE_CRITSECT);
 # else  /* IN_RING0 */
-        int rc = SUPSemEventWaitNoResume(pSession, hEvent, RT_INDEFINITE_WAIT);
+        int rc = SUPSemEventWaitNoResume(pSession, hEvent, RT_MS_5SEC);
 # endif /* IN_RING0 */
 
@@ -189 +195 @@
          * Deal with the return code and critsect destruction.
          */
-        if (RT_UNLIKELY(pCritSect->s.Core.u32Magic != RTCRITSECT_MAGIC))
+        if (RT_LIKELY(pCritSect->s.Core.u32Magic == RTCRITSECT_MAGIC))
+        { /* likely */ }
+        else
             return VERR_SEM_DESTROYED;
         if (rc == VINF_SUCCESS)
             return pdmCritSectEnterFirst(pCritSect, hNativeSelf, pSrcPos);
-        AssertMsg(rc == VERR_INTERRUPTED, ("rc=%Rrc\n", rc));
+        if (RT_LIKELY(rc == VERR_TIMEOUT || rc == VERR_INTERRUPTED))
+        { /* likely */ }
+        else
+        {
+            AssertMsgFailed(("rc=%Rrc\n", rc));
+            return rc;
+        }
 
 # ifdef IN_RING0
@@ -202 +216 @@
                  it without creating a race with PDMCritSectLeave, resulting in
                  spurious wakeups. */
+        RT_NOREF(rcBusy);
+        /** @todo eliminate this and return rcBusy instead.  Guru if
+         *        rcBusy is VINF_SUCCESS. */
         PVMCPUCC pVCpu = VMMGetCpu(pVM); AssertPtr(pVCpu);
         rc = VMMRZCallRing3(pVM, pVCpu, VMMCALLRING3_VM_R0_PREEMPT, NULL);
         AssertRC(rc);
 # else
-        RT_NOREF(pVM);
+        RT_NOREF(pVM, rcBusy);
 # endif
     }
@@ -218 +235 @@
  *
  * @returns VINF_SUCCESS if entered successfully.
- * @returns rcBusy when encountering a busy critical section in GC/R0.
+ * @returns rcBusy when encountering a busy critical section in RC/R0.
  * @retval  VERR_SEM_DESTROYED if the critical section is delete before or
  *          during the operation.
@@ -224 +241 @@
  * @param   pVM                 The cross context VM structure.
  * @param   pCritSect           The PDM critical section to enter.
- * @param   rcBusy              The status code to return when we're in GC or R0
+ * @param   rcBusy              The status code to return when we're in RC or R0
  * @param   pSrcPos             The source position of the lock operation.
  */
@@ -290 +307 @@
      */
     NOREF(rcBusy);
-    return pdmR3R0CritSectEnterContended(pVM, pCritSect, hNativeSelf, pSrcPos);
+    return pdmR3R0CritSectEnterContended(pVM, pCritSect, hNativeSelf, pSrcPos, rcBusy);
 
 #elif defined(IN_RING0)
@@ -314 +331 @@
             Assert(RTThreadPreemptIsEnabled(NIL_RTTHREAD));
 
-            rc = pdmR3R0CritSectEnterContended(pVM, pCritSect, hNativeSelf, pSrcPos);
+            rc = pdmR3R0CritSectEnterContended(pVM, pCritSect, hNativeSelf, pSrcPos, rcBusy);
 
             VMMR0EmtResumeAfterBlocking(pVCpu, &Ctx);
@@ -325 +342 @@
     /* Non-EMT. */
     Assert(RTThreadPreemptIsEnabled(NIL_RTTHREAD));
-    return pdmR3R0CritSectEnterContended(pVM, pCritSect, hNativeSelf, pSrcPos);
+    return pdmR3R0CritSectEnterContended(pVM, pCritSect, hNativeSelf, pSrcPos, rcBusy);
 
 # else /* old code: */
@@ -333 +350 @@
     if (   RTThreadPreemptIsEnabled(NIL_RTTHREAD)
         && ASMIntAreEnabled())
-        return pdmR3R0CritSectEnterContended(pVM, pCritSect, hNativeSelf, pSrcPos);
+        return pdmR3R0CritSectEnterContended(pVM, pCritSect, hNativeSelf, pSrcPos, rcBusy);
 
     STAM_REL_COUNTER_INC(&pCritSect->s.StatContentionRZLock);