[navaraf] 13366: 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.

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;