30 Jan
2005
30 Jan
'05
12:56 p.m.
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;