[jgardou] 57641: [WIN32K] - Properly handle deletion of cursor/Icon objects - Gracefully change object owner when process closes - Remove useless check in a macro

Author: jgardou
Date: Sun Oct 28 14:45:35 2012
New Revision: 57641

URL: http://svn.reactos.org/svn/reactos?rev=57641&view=rev
Log:
[WIN32K]
- Properly handle deletion of cursor/Icon objects
- Gracefully change object owner when process closes
- Remove useless check in a macro

Modified:
    trunk/reactos/win32ss/gdi/ntgdi/misc.h
    trunk/reactos/win32ss/user/ntuser/cursoricon.c
    trunk/reactos/win32ss/user/ntuser/object.c
    trunk/reactos/win32ss/user/ntuser/object.h

Modified: trunk/reactos/win32ss/gdi/ntgdi/misc.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/gdi/ntgdi/misc.h?re...
==============================================================================
--- trunk/reactos/win32ss/gdi/ntgdi/misc.h [iso-8859-1] (original)
+++ trunk/reactos/win32ss/gdi/ntgdi/misc.h [iso-8859-1] Sun Oct 28 14:45:35 2012
@@ -163,6 +163,6 @@
 #define LIST_FOR_EACH_SAFE(cursor, cursor2, list, type, field) \
     for ((cursor) = CONTAINING_RECORD((list)->Flink, type, field), \
          (cursor2) = CONTAINING_RECORD((cursor)->field.Flink, type, field); \
-         &(cursor)->field != (list) && ((&((cursor)->field)) != NULL); \
+         &(cursor)->field != (list); \
          (cursor) = (cursor2), \
          (cursor2) = CONTAINING_RECORD((cursor)->field.Flink, type, field))

Modified: trunk/reactos/win32ss/user/ntuser/cursoricon.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/cursori...
==============================================================================
--- trunk/reactos/win32ss/user/ntuser/cursoricon.c [iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/ntuser/cursoricon.c [iso-8859-1] Sun Oct 28 14:45:35 2012
@@ -234,55 +234,54 @@
 }

BOOLEAN FASTCALL
-IntDestroyCurIconObject(PCURICON_OBJECT CurIcon, BOOL ProcessCleanup)
+IntDestroyCurIconObject(PCURICON_OBJECT CurIcon, PPROCESSINFO ppi)
 {
     PSYSTEM_CURSORINFO CurInfo;
     HBITMAP bmpMask, bmpColor;
-    BOOLEAN Ret;
+    BOOLEAN Ret, bListEmpty, bFound = FALSE;
     PCURICON_PROCESS Current = NULL;
-    PPROCESSINFO W32Process = PsGetCurrentProcessWin32Process();
-
-    /* Private objects can only be destroyed by their own process */
-    if (NULL == CurIcon->hModule)
-    {
-        ASSERT(CurIcon->ProcessList.Flink->Flink == &CurIcon->ProcessList);
-        Current = CONTAINING_RECORD(CurIcon->ProcessList.Flink, CURICON_PROCESS, ListEntry);
-        if (Current->Process != W32Process)
-        {
-            ERR("Trying to destroy private icon/cursor of another process\n");
-            return FALSE;
-        }
-    }
-    else if (! ProcessCleanup)
-    {
-        TRACE("Trying to destroy shared icon/cursor\n");
-        return FALSE;
-    }
+    
+    /* For handles created without any data (error handling) */
+    if(IsListEmpty(&CurIcon->ProcessList))
+        goto emptyList;

/* Now find this process in the list of processes referencing this object and
        remove it from that list */
     LIST_FOR_EACH(Current, &CurIcon->ProcessList, CURICON_PROCESS, ListEntry)
     {
-        if (Current->Process == W32Process)
-        {
-            RemoveEntryList(&Current->ListEntry);
+        if (Current->Process == ppi)
+        {
+            bFound = TRUE;
+            bListEmpty = RemoveEntryList(&Current->ListEntry);
             break;
         }
     }
+    
+    if(!bFound)
+    {
+        /* This object doesn't belong to this process */
+        EngSetLastError(ERROR_INVALID_HANDLE);
+        return FALSE;
+    }

ExFreeToPagedLookasideList(pgProcessLookasideList, Current);

/* If there are still processes referencing this object we can't destroy it yet */
-    if (! IsListEmpty(&CurIcon->ProcessList))
-    {
+    if (!bListEmpty)
+    {
+        if(CurIcon->head.ppi == ppi)
+        {
+            /* Set the first process of the list as owner */
+            Current = CONTAINING_RECORD(CurIcon->ProcessList.Flink, CURICON_PROCESS, ListEntry);
+            UserSetObjectOwner(CurIcon, otCursorIcon, Current->Process);
+        }
+        UserDereferenceObject(CurIcon);
         return TRUE;
     }

-
-    if (! ProcessCleanup)
-    {
-        RemoveEntryList(&CurIcon->ListEntry);
-    }
+emptyList:
+    /* Remove it from the list */
+    RemoveEntryList(&CurIcon->ListEntry);

CurInfo = IntGetSysCursorInfo();

@@ -320,33 +319,13 @@
 IntCleanupCurIcons(struct _EPROCESS *Process, PPROCESSINFO Win32Process)
 {
     PCURICON_OBJECT CurIcon, tmp;
-    PCURICON_PROCESS ProcessData;
-
+
+    /* Run through the list of icon objects */
     LIST_FOR_EACH_SAFE(CurIcon, tmp, &gCurIconList, CURICON_OBJECT, ListEntry)
     {
         UserReferenceObject(CurIcon);
-        //    if(NT_SUCCESS(UserReferenceObjectByPointer(Object, otCursorIcon)))
-        {
-            LIST_FOR_EACH(ProcessData, &CurIcon->ProcessList, CURICON_PROCESS, ListEntry)
-            {
-                if (Win32Process == ProcessData->Process)
-                {
-                    RemoveEntryList(&CurIcon->ListEntry);
-                    IntDestroyCurIconObject(CurIcon, TRUE);
-                    CurIcon = NULL;
-                    break;
-                }
-            }
-
-//         UserDereferenceObject(Object);
-        }
-
-        if (CurIcon)
-        {
-            UserDereferenceObject(CurIcon);
-        }
-    }
-
+        IntDestroyCurIconObject(CurIcon, Win32Process);
+    }
 }

@@ -649,7 +628,7 @@
         RETURN(FALSE);
     }

-    ret = IntDestroyCurIconObject(CurIcon, FALSE);
+    ret = IntDestroyCurIconObject(CurIcon, PsGetCurrentProcessWin32Process());
     /* Note: IntDestroyCurIconObject will remove our reference for us! */

RETURN(ret);

Modified: trunk/reactos/win32ss/user/ntuser/object.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/object....
==============================================================================
--- trunk/reactos/win32ss/user/ntuser/object.c [iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/ntuser/object.c [iso-8859-1] Sun Oct 28 14:45:35 2012
@@ -516,6 +516,38 @@
     return object;
 }

+VOID
+FASTCALL
+UserSetObjectOwner(PVOID obj, USER_OBJECT_TYPE type, PVOID owner)
+{
+    PUSER_HANDLE_ENTRY entry = handle_to_entry(gHandleTable, ((PHEAD)obj)->h );
+    PPROCESSINFO ppi, oldppi;
+    
+    /* This must be called with a valid object */
+    ASSERT(entry);
+    
+    /* For now, only supported for CursorIcon object */
+    switch(type)
+    {
+        case otCursorIcon:
+            ppi = (PPROCESSINFO)owner;
+            entry->pi = ppi;
+            oldppi = ((PPROCMARKHEAD)obj)->ppi;
+            ((PPROCMARKHEAD)obj)->ppi = ppi;
+            break;
+        default:
+            ASSERT(FALSE);
+            return;
+    }
+
+    oldppi->UserHandleCount--;
+    ppi->UserHandleCount++;
+#if DBG
+    oldppi->DbgHandleCount[type]--;
+    ppi->DbgHandleCount[type]++;
+#endif
+}
+
 /*
  * NtUserValidateHandleSecure
  *

Modified: trunk/reactos/win32ss/user/ntuser/object.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/object....
==============================================================================
--- trunk/reactos/win32ss/user/ntuser/object.h [iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/ntuser/object.h [iso-8859-1] Sun Oct 28 14:45:35 2012
@@ -40,6 +40,7 @@
 BOOL FASTCALL UserCreateHandleTable(VOID);
 BOOL FASTCALL UserObjectInDestroy(HANDLE);
 void DbgUserDumpHandleTable();
+VOID FASTCALL UserSetObjectOwner(PVOID obj, USER_OBJECT_TYPE type, PVOID owner);

static __inline VOID
 UserRefObjectCo(PVOID obj, PUSER_REFERENCE_ENTRY UserReferenceEntry)