Attempt to solve the imfamous WM_NCPAINT handle leak. The message handler isn't required to delete the region handle while it's also valid to delete it (eg. by calling GetDCEx with it). Thus we have to send the WM_NCPAINT message only synchronously and verify the handle after the SendMessage call. Modified: trunk/reactos/include/win32k/gdiobj.h Modified: trunk/reactos/subsys/win32k/ntuser/painting.c Modified: trunk/reactos/subsys/win32k/objects/gdiobj.c _____
Modified: trunk/reactos/include/win32k/gdiobj.h --- trunk/reactos/include/win32k/gdiobj.h 2005-01-30 09:46:15 UTC (rev 13365) +++ trunk/reactos/include/win32k/gdiobj.h 2005-01-30 12:56:12 UTC (rev 13366) @@ -51,6 +51,8 @@
#define GDI_OBJECT_TYPE_MEMDC 0x00750000 #define GDI_OBJECT_TYPE_DCE 0x00770000 #define GDI_OBJECT_TYPE_DONTCARE 0x007f0000 +/** Not really an object type. Forces GDI_FreeObj to be silent. */ +#define GDI_OBJECT_TYPE_SILENT 0x80000000 /*@}*/
typedef PVOID PGDIOBJ; _____
Modified: trunk/reactos/subsys/win32k/ntuser/painting.c --- trunk/reactos/subsys/win32k/ntuser/painting.c 2005-01-30 09:46:15 UTC (rev 13365) +++ trunk/reactos/subsys/win32k/ntuser/painting.c 2005-01-30 12:56:12 UTC (rev 13366) @@ -105,6 +105,11 @@
MsqDecPaintCountQueue(Window->MessageQueue); IntUnLockWindowUpdate(Window); IntSendMessage(hWnd, WM_NCPAINT, (WPARAM)TempRegion, 0); + if ((HANDLE) 1 != TempRegion && NULL != TempRegion) + { + /* NOTE: The region can already be deleted! */ + GDIOBJ_FreeObj(TempRegion, GDI_OBJECT_TYPE_REGION | GDI_OBJECT_TYPE_SILENT); + } }
if (Window->Flags & WINDOWOBJECT_NEED_ERASEBKGND) @@ -630,30 +635,10 @@ if (Window != NULL) { IntLockWindowUpdate(Window); - if (0 != (Window->Flags & WINDOWOBJECT_NEED_NCPAINT) - && ((0 == MsgFilterMin && 0 == MsgFilterMax) || - (MsgFilterMin <= WM_NCPAINT && - WM_NCPAINT <= MsgFilterMax))) + + if ((MsgFilterMin == 0 && MsgFilterMax == 0) || + (MsgFilterMin <= WM_PAINT && WM_PAINT <= MsgFilterMax)) { - Message->message = WM_NCPAINT; - Message->wParam = (WPARAM)Window->NCUpdateRegion; - Message->lParam = 0; - if (Remove) - { - if ((HANDLE) 1 != Window->NCUpdateRegion && - NULL != Window->NCUpdateRegion) - { - GDIOBJ_SetOwnership(Window->NCUpdateRegion, PsGetCurrentProcess()); - } - IntValidateParent(Window, Window->NCUpdateRegion); - Window->NCUpdateRegion = NULL; - Window->Flags &= ~WINDOWOBJECT_NEED_NCPAINT; - MsqDecPaintCountQueue(Window->MessageQueue); - } - } else if ((0 == MsgFilterMin && 0 == MsgFilterMax) || - (MsgFilterMin <= WM_PAINT && - WM_PAINT <= MsgFilterMax)) - { Message->message = WM_PAINT; Message->wParam = Message->lParam = 0; if (Remove && Window->Flags & WINDOWOBJECT_NEED_INTERNALPAINT) @@ -737,6 +722,28 @@
NtUserHideCaret(hWnd);
+ if (Window->Flags & WINDOWOBJECT_NEED_NCPAINT) + { + HRGN hRgn; + + if (Window->NCUpdateRegion != (HANDLE)1 && + Window->NCUpdateRegion != NULL) + { + GDIOBJ_SetOwnership(Window->NCUpdateRegion, PsGetCurrentProcess()); + } + hRgn = Window->NCUpdateRegion; + IntValidateParent(Window, Window->NCUpdateRegion); + Window->NCUpdateRegion = NULL; + Window->Flags &= ~WINDOWOBJECT_NEED_NCPAINT; + MsqDecPaintCountQueue(Window->MessageQueue); + IntSendMessage(hWnd, WM_NCPAINT, (WPARAM)hRgn, 0); + if (hRgn != (HANDLE)1 && hRgn != NULL) + { + /* NOTE: The region can already by deleted! */ + GDIOBJ_FreeObj(hRgn, GDI_OBJECT_TYPE_REGION | GDI_OBJECT_TYPE_SILENT); + } + } + RtlZeroMemory(&Ps, sizeof(PAINTSTRUCT)); Ps.hdc = NtUserGetDCEx(hWnd, 0, DCX_INTERSECTUPDATE | DCX_WINDOWPAINT | DCX_USESTYLE); @@ -754,17 +761,17 @@ IntValidateParent(Window, Window->UpdateRegion); Rgn = RGNDATA_LockRgn(Window->UpdateRegion); if (NULL != Rgn) - { - UnsafeIntGetRgnBox(Rgn, &Ps.rcPaint); - RGNDATA_UnlockRgn(Window->UpdateRegion); - IntGdiOffsetRect(&Ps.rcPaint, - Window->WindowRect.left - Window->ClientRect.left, - Window->WindowRect.top - Window->ClientRect.top); - } + { + UnsafeIntGetRgnBox(Rgn, &Ps.rcPaint); + RGNDATA_UnlockRgn(Window->UpdateRegion); + IntGdiOffsetRect(&Ps.rcPaint, + Window->WindowRect.left - Window->ClientRect.left, + Window->WindowRect.top - Window->ClientRect.top); + } else - { - IntGetClientRect(Window, &Ps.rcPaint); - } + { + IntGetClientRect(Window, &Ps.rcPaint); + } GDIOBJ_SetOwnership(Window->UpdateRegion, PsGetCurrentProcess()); NtGdiDeleteObject(Window->UpdateRegion); Window->UpdateRegion = NULL; _____
Modified: trunk/reactos/subsys/win32k/objects/gdiobj.c --- trunk/reactos/subsys/win32k/objects/gdiobj.c 2005-01-30 09:46:15 UTC (rev 13365) +++ trunk/reactos/subsys/win32k/objects/gdiobj.c 2005-01-30 12:56:12 UTC (rev 13366) @@ -467,6 +467,7 @@
PPAGED_LOOKASIDE_LIST LookasideList; HANDLE ProcessId, LockedProcessId, PrevProcId; LONG ExpectedType; + BOOL Silent; #ifdef GDI_DEBUG ULONG Attempts = 0; #endif @@ -485,6 +486,9 @@ ProcessId = PsGetCurrentProcessId(); LockedProcessId = (HANDLE)((ULONG_PTR)ProcessId | 0x1);
+ Silent = (ObjectType & GDI_OBJECT_TYPE_SILENT); + ObjectType &= ~GDI_OBJECT_TYPE_SILENT; + ExpectedType = ((ObjectType != GDI_OBJECT_TYPE_DONTCARE) ? ObjectType : 0);
Entry = GDI_HANDLE_GET_ENTRY(HandleTable, hObj); @@ -577,17 +581,20 @@ } else { - if(((ULONG_PTR)PrevProcId & 0x1) == 0) + if(!Silent) { - DPRINT1("Attempted to free global gdi handle 0x%x, caller needs to get ownership first!!!", hObj); - } - else - { - DPRINT1("Attempted to free foreign handle: 0x%x Owner: 0x%x from Caller: 0x%x\n", hObj, (ULONG_PTR)PrevProcId & ~0x1, (ULONG_PTR)ProcessId & ~0x1); - } + if(((ULONG_PTR)PrevProcId & ~0x1) == 0) + { + DPRINT1("Attempted to free global gdi handle 0x%x, caller needs to get ownership first!!!\n", hObj); + } + else + { + DPRINT1("Attempted to free foreign handle: 0x%x Owner: 0x%x from Caller: 0x%x\n", hObj, (ULONG_PTR)PrevProcId & ~0x1, (ULONG_PTR)ProcessId & ~0x1); + } #ifdef GDI_DEBUG - DPRINT1("-> called from %s:%i\n", file, line); + DPRINT1("-> called from %s:%i\n", file, line); #endif + } }
return FALSE;