[ros-diffs] [ion] 26322: - Fix several pushlock bugs (thanks in part to hto). Should fix bug 2063.
ion at svn.reactos.org
ion at svn.reactos.org
Wed Apr 11 22:00:54 CEST 2007
Author: ion
Date: Thu Apr 12 00:00:53 2007
New Revision: 26322
URL: http://svn.reactos.org/svn/reactos?rev=26322&view=rev
Log:
- Fix several pushlock bugs (thanks in part to hto). Should fix bug 2063.
Modified:
trunk/reactos/ntoskrnl/ex/pushlock.c
Modified: trunk/reactos/ntoskrnl/ex/pushlock.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ex/pushlock.c?rev=26322&r1=26321&r2=26322&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ex/pushlock.c (original)
+++ trunk/reactos/ntoskrnl/ex/pushlock.c Thu Apr 12 00:00:53 2007
@@ -76,31 +76,24 @@
ASSERT(!OldValue.MultipleShared);
/* Check if it's locked */
- if (OldValue.Locked)
- {
- /* If it's locked we must simply un-wake it*/
- for (;;)
- {
- /* It's not waking anymore */
- NewValue.Value = OldValue.Value &~ EX_PUSH_LOCK_WAKING;
-
- /* Sanity checks */
- ASSERT(!NewValue.Waking);
- ASSERT(NewValue.Locked);
- ASSERT(NewValue.Waiting);
-
- /* Write the New Value */
- NewValue.Ptr = InterlockedCompareExchangePointer(PushLock,
- NewValue.Ptr,
- OldValue.Ptr);
- if (NewValue.Value == OldValue.Value) return;
-
- /* Someone changed the value behind our back, update it*/
- OldValue = NewValue;
-
- /* Check if it's still locked */
- if (!OldValue.Locked) break;
- }
+ while (OldValue.Locked)
+ {
+ /* It's not waking anymore */
+ NewValue.Value = OldValue.Value &~ EX_PUSH_LOCK_WAKING;
+
+ /* Sanity checks */
+ ASSERT(!NewValue.Waking);
+ ASSERT(NewValue.Locked);
+ ASSERT(NewValue.Waiting);
+
+ /* Write the New Value */
+ NewValue.Ptr = InterlockedCompareExchangePointer(PushLock,
+ NewValue.Ptr,
+ OldValue.Ptr);
+ if (NewValue.Value == OldValue.Value) return;
+
+ /* Someone changed the value behind our back, update it*/
+ OldValue = NewValue;
}
/* Save the First Block */
@@ -131,8 +124,17 @@
!(PreviousWaitBlock))
{
/* Destroy the pushlock */
- if (InterlockedCompareExchangePointer(PushLock, 0, OldValue.Ptr) ==
- OldValue.Ptr) break;
+ NewValue.Value = 0;
+ ASSERT(!NewValue.Waking);
+
+ /* Write the New Value */
+ NewValue.Ptr = InterlockedCompareExchangePointer(PushLock,
+ NewValue.Ptr,
+ OldValue.Ptr);
+ if (NewValue.Value == OldValue.Value) break;
+
+ /* Someone changed the value behind our back, update it*/
+ OldValue = NewValue;
}
else
{
@@ -224,7 +226,7 @@
/* Loop the blocks */
LastWaitBlock = WaitBlock->Last;
- while (LastWaitBlock)
+ while (!LastWaitBlock)
{
/* Save the block */
PreviousWaitBlock = WaitBlock;
@@ -393,13 +395,13 @@
{
PEX_PUSH_LOCK_WAIT_BLOCK WaitBlock = pWaitBlock;
EX_PUSH_LOCK NewValue, OldValue;
-
+
/* Detect invalid wait block alignment */
- ASSERT((ULONG_PTR)pWaitBlock & 0x10);
+ ASSERT(((ULONG_PTR)pWaitBlock & 0xF) == 0);
/* Set the waiting bit */
WaitBlock->Flags = EX_PUSH_LOCK_FLAGS_WAIT;
-
+
/* Get the old value */
OldValue = *PushLock;
@@ -414,7 +416,7 @@
WaitBlock,
OldValue.Ptr);
if (OldValue.Ptr == NewValue.Ptr) break;
-
+
/* Try again with the new value */
NewValue = OldValue;
}
@@ -492,10 +494,10 @@
/* Point to ours */
NewValue.Value = (OldValue.Value & EX_PUSH_LOCK_MULTIPLE_SHARED) |
- EX_PUSH_LOCK_LOCK |
- EX_PUSH_LOCK_WAKING |
- EX_PUSH_LOCK_WAITING |
- PtrToUlong(&WaitBlock);
+ EX_PUSH_LOCK_LOCK |
+ EX_PUSH_LOCK_WAKING |
+ EX_PUSH_LOCK_WAITING |
+ PtrToUlong(WaitBlock);
/* Check if the pushlock was already waking */
if (OldValue.Waking) NeedWake = TRUE;
@@ -515,7 +517,7 @@
NewValue.Value = EX_PUSH_LOCK_MULTIPLE_SHARED |
EX_PUSH_LOCK_LOCK |
EX_PUSH_LOCK_WAITING |
- PtrToUlong(&WaitBlock);
+ PtrToUlong(WaitBlock);
}
else
{
@@ -525,7 +527,7 @@
/* Point to our wait block */
NewValue.Value = EX_PUSH_LOCK_LOCK |
EX_PUSH_LOCK_WAITING |
- PtrToUlong(&WaitBlock);
+ PtrToUlong(WaitBlock);
}
}
@@ -589,10 +591,10 @@
}
/*++
- * @name ExAcquirePushLockExclusive
+ * @name ExAcquirePushLockShared
* @implemented NT5.1
*
- * The ExAcquirePushLockShared macro acquires a shared PushLock.
+ * The ExAcquirePushLockShared routine acquires a shared PushLock.
*
* @params PushLock
* Pointer to the pushlock which is to be acquired.
@@ -616,7 +618,7 @@
for (;;)
{
/* Check if it's unlocked or if it's waiting without any sharers */
- if (!(OldValue.Locked) || (OldValue.Waiting && OldValue.Shared == 0))
+ if (!(OldValue.Locked) || (!(OldValue.Waiting) && (OldValue.Shared > 0)))
{
/* Check if anyone is waiting on it */
if (!OldValue.Waiting)
@@ -671,10 +673,10 @@
EX_PUSH_LOCK_LOCK)) |
EX_PUSH_LOCK_WAKING |
EX_PUSH_LOCK_WAITING |
- PtrToUlong(&WaitBlock);
+ PtrToUlong(WaitBlock);
/* Check if the pushlock was already waking */
- if (OldValue.Waking) NeedWake = TRUE;
+ if (!OldValue.Waking) NeedWake = TRUE;
}
else
{
@@ -682,10 +684,9 @@
WaitBlock->Last = WaitBlock;
/* Point to our wait block */
- NewValue.Value = (OldValue.Value & (EX_PUSH_LOCK_MULTIPLE_SHARED |
- EX_PUSH_LOCK_WAKING)) |
+ NewValue.Value = (OldValue.Value & EX_PUSH_LOCK_PTR_BITS) |
EX_PUSH_LOCK_WAITING |
- PtrToUlong(&WaitBlock);
+ PtrToUlong(WaitBlock);
}
/* Sanity check */
@@ -700,9 +701,10 @@
#endif
/* Write the new value */
- if (InterlockedCompareExchangePointer(PushLock,
- NewValue.Ptr,
- OldValue.Ptr) != OldValue.Ptr)
+ NewValue.Ptr = InterlockedCompareExchangePointer(PushLock,
+ NewValue.Ptr,
+ OldValue.Ptr);
+ if (NewValue.Ptr != OldValue.Ptr)
{
/* Retry */
OldValue = NewValue;
@@ -748,15 +750,15 @@
* @name ExfReleasePushLock
* @implemented NT5.1
*
- * The ExReleasePushLockExclusive routine releases a previously
- * exclusively acquired PushLock.
+ * The ExReleasePushLock routine releases a previously acquired PushLock.
+ *
*
* @params PushLock
* Pointer to a previously acquired pushlock.
*
* @return None.
*
- * @remarks Callers of ExReleasePushLockExclusive must be running at IRQL <= APC_LEVEL.
+ * @remarks Callers of ExfReleasePushLock must be running at IRQL <= APC_LEVEL.
* This macro should usually be paired up with KeLeaveCriticalRegion.
*
*--*/
@@ -794,11 +796,7 @@
NewValue.Ptr = InterlockedCompareExchangePointer(PushLock,
NewValue.Ptr,
OldValue.Ptr);
- if (NewValue.Value == OldValue.Value)
- {
- /* No waiters left, we're done */
- goto quit;
- }
+ if (NewValue.Value == OldValue.Value) return;
/* Did it enter a wait state? */
OldValue = NewValue;
@@ -812,21 +810,17 @@
/* Find the last Wait Block */
for (WaitBlock = (PEX_PUSH_LOCK_WAIT_BLOCK)((ULONG_PTR)OldValue.Ptr &
~EX_PUSH_LOCK_PTR_BITS);
- WaitBlock->Last;
+ !WaitBlock->Last;
WaitBlock = WaitBlock->Next);
/* Make sure the Share Count is above 0 */
- if (WaitBlock->ShareCount)
+ if (WaitBlock->ShareCount > 0)
{
/* This shouldn't be an exclusive wait block */
- ASSERT(WaitBlock->Flags&EX_PUSH_LOCK_FLAGS_EXCLUSIVE);
+ ASSERT(WaitBlock->Flags & EX_PUSH_LOCK_FLAGS_EXCLUSIVE);
/* Do the decrease and check if the lock isn't shared anymore */
- if (InterlockedExchangeAdd(&WaitBlock->ShareCount, -1))
- {
- /* Someone is still holding the lock */
- goto quit;
- }
+ if (InterlockedDecrement(&WaitBlock->ShareCount) > 0) return;
}
}
@@ -854,10 +848,7 @@
NewValue.Ptr = InterlockedCompareExchangePointer(PushLock,
NewValue.Ptr,
OldValue.Ptr);
- if (NewValue.Value == OldValue.Value) break;
-
- /* The value changed, try the unlock again */
- continue;
+ if (NewValue.Value == OldValue.Value) return;
}
else
{
@@ -880,12 +871,9 @@
/* The write was successful. The pushlock is Unlocked and Waking */
ExfWakePushLock(PushLock, NewValue);
- break;
- }
- }
-quit:
- /* Done! */
- return;
+ return;
+ }
+ }
}
/*++
@@ -912,38 +900,29 @@
PEX_PUSH_LOCK_WAIT_BLOCK WaitBlock;
/* Check if someone is waiting on the lock */
- if (!OldValue.Waiting)
- {
- /* Nobody is waiting on it, so we'll try a quick release */
- for (;;)
- {
- /* Check if it's shared */
- if (OldValue.Shared > 1)
- {
- /* Write the Old Value but decrease share count */
- NewValue = OldValue;
- NewValue.Shared--;
- }
- else
- {
- /* Simply clear the lock */
- NewValue.Value = 0;
- }
-
- /* Write the New Value */
- NewValue.Ptr = InterlockedCompareExchangePointer(PushLock,
- NewValue.Ptr,
- OldValue.Ptr);
- if (NewValue.Value == OldValue.Value)
- {
- /* No waiters left, we're done */
- goto quit;
- }
-
- /* Did it enter a wait state? */
- OldValue = NewValue;
- if (NewValue.Waiting) break;
- }
+ while (!OldValue.Waiting)
+ {
+ /* Check if it's shared */
+ if (OldValue.Shared > 1)
+ {
+ /* Write the Old Value but decrease share count */
+ NewValue = OldValue;
+ NewValue.Shared--;
+ }
+ else
+ {
+ /* Simply clear the lock */
+ NewValue.Value = 0;
+ }
+
+ /* Write the New Value */
+ NewValue.Ptr = InterlockedCompareExchangePointer(PushLock,
+ NewValue.Ptr,
+ OldValue.Ptr);
+ if (NewValue.Value == OldValue.Value) return;
+
+ /* Did it enter a wait state? */
+ OldValue = NewValue;
}
/* Ok, we do know someone is waiting on it. Are there more then one? */
@@ -952,15 +931,15 @@
/* Find the last Wait Block */
for (WaitBlock = (PEX_PUSH_LOCK_WAIT_BLOCK)((ULONG_PTR)OldValue.Ptr &
~EX_PUSH_LOCK_PTR_BITS);
- WaitBlock->Last;
+ !WaitBlock->Last;
WaitBlock = WaitBlock->Next);
/* Sanity checks */
ASSERT(WaitBlock->ShareCount > 0);
- ASSERT(WaitBlock->Flags&EX_PUSH_LOCK_FLAGS_EXCLUSIVE);
+ ASSERT(WaitBlock->Flags & EX_PUSH_LOCK_FLAGS_EXCLUSIVE);
/* Do the decrease and check if the lock isn't shared anymore */
- if (InterlockedExchangeAdd(&WaitBlock->ShareCount, -1)) goto quit;
+ if (InterlockedDecrement(&WaitBlock->ShareCount) > 0) return;
}
/*
@@ -987,10 +966,7 @@
NewValue.Ptr = InterlockedCompareExchangePointer(PushLock,
NewValue.Ptr,
OldValue.Ptr);
- if (NewValue.Value == OldValue.Value) break;
-
- /* The value changed, try the unlock again */
- continue;
+ if (NewValue.Value == OldValue.Value) return;
}
else
{
@@ -1013,12 +989,9 @@
/* The write was successful. The pushlock is Unlocked and Waking */
ExfWakePushLock(PushLock, NewValue);
- break;
- }
- }
-quit:
- /* Done! */
- return;
+ return;
+ }
+ }
}
/*++
More information about the Ros-diffs
mailing list