From fe2f2f66fc6ba6e0636806ad5eb04bbcdbf7a881 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Sun, 30 Jan 2005 12:56:12 +0000 Subject: [PATCH] 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. svn path=/trunk/; revision=13366 --- reactos/include/win32k/gdiobj.h | 2 + reactos/subsys/win32k/ntuser/painting.c | 73 ++++++++++++++----------- reactos/subsys/win32k/objects/gdiobj.c | 23 +++++--- 3 files changed, 57 insertions(+), 41 deletions(-) diff --git a/reactos/include/win32k/gdiobj.h b/reactos/include/win32k/gdiobj.h index 71c699a6e8d..8d94dfa85a2 100644 --- a/reactos/include/win32k/gdiobj.h +++ b/reactos/include/win32k/gdiobj.h @@ -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; diff --git a/reactos/subsys/win32k/ntuser/painting.c b/reactos/subsys/win32k/ntuser/painting.c index 3dd765ba674..9fb5e70aecd 100644 --- a/reactos/subsys/win32k/ntuser/painting.c +++ b/reactos/subsys/win32k/ntuser/painting.c @@ -105,6 +105,11 @@ IntPaintWindows(PWINDOW_OBJECT Window, ULONG Flags) 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,29 +635,9 @@ IntGetPaintMessage(HWND hWnd, UINT MsgFilterMin, UINT MsgFilterMax, if (Window != NULL) { IntLockWindowUpdate(Window); - if (0 != (Window->Flags & WINDOWOBJECT_NEED_NCPAINT) - && ((0 == MsgFilterMin && 0 == MsgFilterMax) || - (MsgFilterMin <= WM_NCPAINT && - WM_NCPAINT <= 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)) + + if ((MsgFilterMin == 0 && MsgFilterMax == 0) || + (MsgFilterMin <= WM_PAINT && WM_PAINT <= MsgFilterMax)) { Message->message = WM_PAINT; Message->wParam = Message->lParam = 0; @@ -737,6 +722,28 @@ NtUserBeginPaint(HWND hWnd, PAINTSTRUCT* UnsafePs) 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 @@ NtUserBeginPaint(HWND hWnd, PAINTSTRUCT* UnsafePs) 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; diff --git a/reactos/subsys/win32k/objects/gdiobj.c b/reactos/subsys/win32k/objects/gdiobj.c index 0be5dc8e9d7..79902d92b1e 100644 --- a/reactos/subsys/win32k/objects/gdiobj.c +++ b/reactos/subsys/win32k/objects/gdiobj.c @@ -467,6 +467,7 @@ GDIOBJ_FreeObj(HGDIOBJ hObj, DWORD ObjectType) PPAGED_LOOKASIDE_LIST LookasideList; HANDLE ProcessId, LockedProcessId, PrevProcId; LONG ExpectedType; + BOOL Silent; #ifdef GDI_DEBUG ULONG Attempts = 0; #endif @@ -485,6 +486,9 @@ GDIOBJ_FreeObj(HGDIOBJ hObj, DWORD ObjectType) 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 @@ LockHandle: } 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;