Author: tkreuzer
Date: Thu Feb 3 19:25:09 2011
New Revision: 50604
URL: http://svn.reactos.org/svn/reactos?rev=50604&view=rev
Log:
[WIN32K]
Fix buggy mechanism of pushing and popping free gdi handle slots. The old
mechanism unneccessarily locked the entry and it was prone to the ABA problem
as it didn't use a sequence number.
Modified:
trunk/reactos/subsystems/win32/win32k/objects/gdiobj.c
Modified: trunk/reactos/subsystems/win32/win32k/objects/gdiobj.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/objects/gdiobj.c?rev=50604&r1=50603&r2=50604&view=diff
==============================================================================
--- trunk/reactos/subsystems/win32/win32k/objects/gdiobj.c [iso-8859-1]
(original)
+++ trunk/reactos/subsystems/win32/win32k/objects/gdiobj.c [iso-8859-1] Thu Feb
3 19:25:09 2011
@@ -272,64 +272,51 @@
FASTCALL
InterlockedPopFreeEntry(VOID)
{
- ULONG idxFirst, idxNext, idxPrev;
+ ULONG iFirst, iNext, iPrev;
PGDI_TABLE_ENTRY pEntry;
- DWORD PrevProcId;
DPRINT("Enter InterLockedPopFreeEntry\n");
- while (TRUE)
- {
- idxFirst = GdiHandleTable->FirstFree;
-
- if (!idxFirst)
+ do
+ {
+ /* Get the index and sequence number of the first free entry */
+ iFirst = GdiHandleTable->FirstFree;
+
+ /* Check if we have a free entry */
+ if (!(iFirst & GDI_HANDLE_INDEX_MASK))
{
/* Increment FirstUnused and get the new index */
- idxFirst =
InterlockedIncrement((LONG*)&GdiHandleTable->FirstUnused) - 1;
-
- /* Check if we have entries left */
- if (idxFirst >= GDI_HANDLE_COUNT)
+ iFirst = InterlockedIncrement((LONG*)&GdiHandleTable->FirstUnused)
- 1;
+
+ /* Check if we have unused entries left */
+ if (iFirst >= GDI_HANDLE_COUNT)
{
DPRINT1("No more gdi handles left!\n");
return 0;
}
/* Return the old index */
- return idxFirst;
+ return iFirst;
}
/* Get a pointer to the first free entry */
- pEntry = GdiHandleTable->Entries + idxFirst;
-
- /* Try to lock the entry */
- PrevProcId = InterlockedCompareExchange((LONG*)&pEntry->ProcessId, 1,
0);
- if (PrevProcId != 0)
- {
- /* The entry was locked or not free, wait and start over */
- DelayExecution();
- continue;
- }
-
- /* Sanity check: is entry really free? */
- ASSERT(((ULONG_PTR)pEntry->KernelData & ~GDI_HANDLE_INDEX_MASK) == 0);
+ pEntry = GdiHandleTable->Entries + (iFirst & GDI_HANDLE_INDEX_MASK);
+
+ /* Create a new value with an increased sequence number */
+ iNext = (USHORT)(ULONG_PTR)pEntry->KernelData;
+ iNext |= (iFirst & ~GDI_HANDLE_INDEX_MASK) + 0x10000;
/* Try to exchange the FirstFree value */
- idxNext = (ULONG_PTR)pEntry->KernelData;
- idxPrev = InterlockedCompareExchange((LONG*)&GdiHandleTable->FirstFree,
- idxNext,
- idxFirst);
-
- /* Unlock the free entry */
- (void)InterlockedExchange((LONG*)&pEntry->ProcessId, 0);
-
- /* If we succeeded, break out of the loop */
- if (idxPrev == idxFirst)
- {
- break;
- }
- }
-
- return idxFirst;
+ iPrev = InterlockedCompareExchange((LONG*)&GdiHandleTable->FirstFree,
+ iNext,
+ iFirst);
+ }
+ while (iPrev != iFirst);
+
+ /* Sanity check: is entry really free? */
+ ASSERT(((ULONG_PTR)pEntry->KernelData & ~GDI_HANDLE_INDEX_MASK) == 0);
+
+ return iFirst & GDI_HANDLE_INDEX_MASK;
}
/* Pushes an entry of the handle table to the free list,
@@ -338,7 +325,7 @@
FASTCALL
InterlockedPushFreeEntry(ULONG idxToFree)
{
- ULONG idxFirstFree, idxPrev;
+ ULONG iToFree, iFirst, iPrev;
PGDI_TABLE_ENTRY pFreeEntry;
DPRINT("Enter InterlockedPushFreeEntry\n");
@@ -346,18 +333,26 @@
pFreeEntry = GdiHandleTable->Entries + idxToFree;
ASSERT((pFreeEntry->Type & GDI_ENTRY_BASETYPE_MASK) == 0);
ASSERT(pFreeEntry->ProcessId == 0);
- pFreeEntry->UserData = NULL;
+ pFreeEntry->UserData = NULL; // FIXME
+ ASSERT(pFreeEntry->UserData == NULL);
do
{
- idxFirstFree = GdiHandleTable->FirstFree;
- pFreeEntry->KernelData = (PVOID)(ULONG_PTR)idxFirstFree;
-
- idxPrev = InterlockedCompareExchange((LONG*)&GdiHandleTable->FirstFree,
- idxToFree,
- idxFirstFree);
- }
- while (idxPrev != idxFirstFree);
+ /* Get the current first free index and sequence number */
+ iFirst = GdiHandleTable->FirstFree;
+
+ /* Set the KernelData member to the index of the first free entry */
+ pFreeEntry->KernelData = UlongToPtr(iFirst & GDI_HANDLE_INDEX_MASK);
+
+ /* Combine new index and increased sequence number in iToFree */
+ iToFree = idxToFree | ((iFirst & ~GDI_HANDLE_INDEX_MASK) + 0x10000);
+
+ /* Try to atomically update the first free entry */
+ iPrev = InterlockedCompareExchange((LONG*)&GdiHandleTable->FirstFree,
+ iToFree,
+ iFirst);
+ }
+ while (iPrev != iFirst);
}
