- Fix locking bugs in guarded mutex implementation. In race conditions some operations were not re-attempted.

- Fix some other logic bugs, including a serious bug in KeTrytoAcquireGuardedMutex which inversed the result.

svn path=/trunk/; revision=25473

reactos/ntoskrnl/include/internal/ex.h

@@ -745,7 +745,8 @@ ExReleasePushLockExclusive(PEX_PUSH_LOCK PushLock)
     ASSERT(PushLock->Waiting || PushLock->Shared == 0);
 
     /* Unlock the pushlock */
-    OldValue.Value = InterlockedExchangeAddSizeT((PLONG)PushLock, -1);
+    OldValue.Value = InterlockedExchangeAddSizeT((PLONG)PushLock,
+                                                 -EX_PUSH_LOCK_LOCK);
 
     /* Sanity checks */
     ASSERT(OldValue.Locked);

reactos/ntoskrnl/ke/gate.c

@@ -78,7 +78,7 @@ KeWaitForGate(IN PKGATE Gate,
 
                 /* Release the APC lock and return */
                 KiReleaseApcLock(&ApcLock);
-                return;
+                break;
             }
 
             /* Setup a Wait Block */
@@ -122,8 +122,8 @@ KeWaitForGate(IN PKGATE Gate,
             /* Find a new thread to run */
             Status = KiSwapThread(Thread, KeGetCurrentPrcb());
 
-            /* Check if we were executing an APC */
+            /* Make sure we weren't executing an APC */
-            if (Status != STATUS_KERNEL_APC) return;
+            if (Status == STATUS_SUCCESS) return;
         }
     } while (TRUE);
 }
@@ -149,18 +149,14 @@ KeSignalGateBoostPriority(IN PKGATE Gate)
         KiAcquireDispatcherObject(&Gate->Header);
 
         /* Make sure we're not already signaled or that the list is empty */
-        if (Gate->Header.SignalState)
+        if (Gate->Header.SignalState) break;
-        {
-            /* Lower IRQL and quit */
-            KeLowerIrql(OldIrql);
-            return;
-        }
 
         /* Check if our wait list is empty */
         if (IsListEmpty(&Gate->Header.WaitListHead))
         {
             /* It is, so signal the event */
             Gate->Header.SignalState = 1;
+            break;
         }
         else
         {
@@ -187,7 +183,7 @@ KeSignalGateBoostPriority(IN PKGATE Gate)
             RemoveEntryList(&WaitBlock->WaitListEntry);
 
             /* Clear wait status */
-            WaitThread->WaitStatus = 0;
+            WaitThread->WaitStatus = STATUS_SUCCESS;
 
             /* Set state and CPU */
             WaitThread->State = DeferredReady;
@@ -223,6 +219,7 @@ KeSignalGateBoostPriority(IN PKGATE Gate)
 
             /* Exit the dispatcher */
             KiExitDispatcher(OldIrql);
+            return;
         }
     }
 

reactos/ntoskrnl/ke/gmutex.c

@@ -30,9 +30,6 @@ KiAcquireGuardedMutexContented(IN OUT PKGUARDED_MUTEX GuardedMutex)
     BitsToRemove = GM_LOCK_BIT;
     BitsToAdd = GM_LOCK_WAITER_INC;
 
-    /* Get the Count Bits */
-    OldValue = GuardedMutex->Count;
-
     /* Start change loop */
     for (;;)
     {
@@ -42,43 +39,47 @@ KiAcquireGuardedMutexContented(IN OUT PKGUARDED_MUTEX GuardedMutex)
         ASSERT((BitsToAdd == GM_LOCK_WAITER_INC) ||
                (BitsToAdd == GM_LOCK_WAITER_WOKEN));
 
-        /* Check if the Guarded Mutex is locked */
+        /* Get the Count Bits */
-        if (OldValue & GM_LOCK_BIT)
+        OldValue = GuardedMutex->Count;
-        {
-            /* Sanity check */
-            ASSERT((BitsToRemove == GM_LOCK_BIT) ||
-                   ((OldValue & GM_LOCK_WAITER_WOKEN) != 0));
 
-            /* Unlock it by removing the Lock Bit */
+        /* Start internal bit change loop */
-            NewValue = InterlockedCompareExchange(&GuardedMutex->Count,
+        for (;;)
-                                                  OldValue ^ BitsToRemove,
-                                                  OldValue);
-            if (NewValue == OldValue) break;
-
-            /* Value got changed behind our backs, start over */
-            OldValue = NewValue;
-        }
-        else
         {
-            /* The Guarded Mutex isn't locked, so simply set the bits */
+            /* Check if the Guarded Mutex is locked */
-            NewValue = InterlockedCompareExchange(&GuardedMutex->Count,
+            if (OldValue & GM_LOCK_BIT)
-                                                  OldValue + BitsToAdd,
-                                                  OldValue);
-            if (NewValue != OldValue)
             {
-                /* Value got changed behind our backs, start over */
+                /* Sanity check */
-                OldValue = NewValue;
+                ASSERT((BitsToRemove == GM_LOCK_BIT) ||
-                continue;
+                       ((OldValue & GM_LOCK_WAITER_WOKEN) != 0));
+
+                /* Unlock it by removing the Lock Bit */
+                NewValue = OldValue ^ BitsToRemove;
+                NewValue = InterlockedCompareExchange(&GuardedMutex->Count,
+                                                      NewValue,
+                                                      OldValue);
+                if (NewValue == OldValue) return;
+            }
+            else
+            {
+                /* The Guarded Mutex isn't locked, so simply set the bits */
+                NewValue = OldValue + BitsToAdd;
+                NewValue = InterlockedCompareExchange(&GuardedMutex->Count,
+                                                      NewValue,
+                                                      OldValue);
+                if (NewValue == OldValue) break;
             }
 
-            /* Now we have to wait for it */
+            /* Old value changed, loop again */
-            KeWaitForGate(&GuardedMutex->Gate, WrGuardedMutex, KernelMode);
+            OldValue = NewValue;
-            ASSERT((GuardedMutex->Count & GM_LOCK_WAITER_WOKEN) != 0);
+        }
 
-            /* Ok, the wait is done, so set the new bits */
+        /* Now we have to wait for it */
-            BitsToRemove = GM_LOCK_BIT | GM_LOCK_WAITER_WOKEN;
+        KeWaitForGate(&GuardedMutex->Gate, WrGuardedMutex, KernelMode);
-            BitsToAdd = GM_LOCK_WAITER_WOKEN;
+        ASSERT((GuardedMutex->Count & GM_LOCK_WAITER_WOKEN) != 0);
-       }
+
+        /* Ok, the wait is done, so set the new bits */
+        BitsToRemove = GM_LOCK_BIT | GM_LOCK_WAITER_WOKEN;
+        BitsToAdd = GM_LOCK_WAITER_WOKEN;
     }
 }
 
@@ -109,7 +110,7 @@ KiReleaseGuardedMutex(IN OUT PKGUARDED_MUTEX GuardedMutex)
     GuardedMutex->Owner = NULL;
 
     /* Add the Lock Bit */
-    OldValue = InterlockedExchangeAdd(&GuardedMutex->Count, 1);
+    OldValue = InterlockedExchangeAdd(&GuardedMutex->Count, GM_LOCK_BIT);
     ASSERT((OldValue & GM_LOCK_BIT) == 0);
 
     /* Check if it was already locked, but not woken */
@@ -247,7 +248,7 @@ KeTryToAcquireGuardedMutex(IN OUT PKGUARDED_MUTEX GuardedMutex)
 
     /* Remove the lock */
     OldBit = InterlockedBitTestAndReset(&GuardedMutex->Count, GM_LOCK_BIT_V);
-    if (OldBit)
+    if (!OldBit)
     {
         /* Re-enable APCs */
         KeLeaveGuardedRegion();