[jgardou] 64912: [WIN32K] - Remove the cursor from the process cache when its handle is deleted, not after. - Do not return an invalid handle in NtUserSetCursor. CORE-7575

Author: jgardou
Date: Thu Oct 23 09:32:39 2014
New Revision: 64912

URL: http://svn.reactos.org/svn/reactos?rev=64912&view=rev
Log:
[WIN32K]
 - Remove the cursor from the process cache when its handle is deleted, not after.
 - Do not return an invalid handle in NtUserSetCursor.
CORE-7575

Modified:
    trunk/reactos/win32ss/user/ntuser/cursoricon_new.c

Modified: trunk/reactos/win32ss/user/ntuser/cursoricon_new.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/cursori...
==============================================================================
--- trunk/reactos/win32ss/user/ntuser/cursoricon_new.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/ntuser/cursoricon_new.c	[iso-8859-1] Thu Oct 23 09:32:39 2014
@@ -63,9 +63,9 @@
         return NULL;
     }

-    if(UserObjectInDestroy(hCurIcon))
-    {
-        ERR("Requesting destroyed cursor.\n");
+    if (UserObjectInDestroy(hCurIcon))
+    {
+        WARN("Requesting invalid/destroyed cursor.\n");
         EngSetLastError(ERROR_INVALID_CURSOR_HANDLE);
         return NULL;
     }
@@ -155,71 +155,20 @@
 {
     PCURICON_OBJECT CurIcon = Object;

-    /* We just mark the handle as being destroyed.
-     * Deleting all the stuff will be deferred to the actual struct free. */
-    return UserDeleteObject(CurIcon->head.h, TYPE_CURSOR);
-}
-
-void
-FreeCurIconObject(
-    _In_ PVOID Object)
-{
-    PCURICON_OBJECT CurIcon = Object;
-    
-    if(!(CurIcon->CURSORF_flags & CURSORF_ACON))
-    {
-        HBITMAP bmpMask = CurIcon->hbmMask;
-        HBITMAP bmpColor = CurIcon->hbmColor;
-        HBITMAP bmpAlpha = CurIcon->hbmAlpha;
-
-        /* Delete bitmaps */
-        if (bmpMask)
-        {
-            GreSetObjectOwner(bmpMask, GDI_OBJ_HMGR_POWNED);
-            GreDeleteObject(bmpMask);
-            CurIcon->hbmMask = NULL;
-        }
-        if (bmpColor)
-        {
-            GreSetObjectOwner(bmpColor, GDI_OBJ_HMGR_POWNED);
-            GreDeleteObject(bmpColor);
-            CurIcon->hbmColor = NULL;
-        }
-        if (bmpAlpha)
-        {
-            GreSetObjectOwner(bmpAlpha, GDI_OBJ_HMGR_POWNED);
-            GreDeleteObject(bmpAlpha);
-            CurIcon->hbmAlpha = NULL;
-        }
-    }
-    else
-    {
-        PACON AniCurIcon = (PACON)CurIcon;
-        UINT i;
-
-        for(i = 0; i < AniCurIcon->cpcur; i++)
-            IntDestroyCurIconObject(AniCurIcon->aspcur[i]);
-        ExFreePoolWithTag(AniCurIcon->aspcur, USERTAG_CURSOR);
-    }
-
+    /* Try finding it in its process cache */
     if (CurIcon->CURSORF_flags & CURSORF_LRSHARED)
     {
         PPROCESSINFO ppi;

-        if (!IS_INTRESOURCE(CurIcon->strName.Buffer))
-            ExFreePoolWithTag(CurIcon->strName.Buffer, TAG_STRING);
-        if (CurIcon->atomModName)
-            RtlDeleteAtomFromAtomTable(gAtomTable, CurIcon->atomModName);
-        CurIcon->strName.Buffer = NULL;
-        CurIcon->atomModName = 0;
-
-        /* Try finding it in its process cache */
         ppi = CurIcon->head.ppi;
         if (ppi->pCursorCache == CurIcon)
+        {
             ppi->pCursorCache = CurIcon->pcurNext;
+            UserDereferenceObject(CurIcon);
+        }
         else
         {
-            PCURICON_OBJECT CacheCurIcon= ppi->pCursorCache;
+            PCURICON_OBJECT CacheCurIcon = ppi->pCursorCache;
             while (CacheCurIcon)
             {
                 if (CacheCurIcon->pcurNext == CurIcon)
@@ -229,7 +178,68 @@
                 }
                 CacheCurIcon = CacheCurIcon->pcurNext;
             }
-        }
+
+            /* We must have found it! */
+            ASSERT(CacheCurIcon != NULL);
+            UserDereferenceObject(CurIcon);
+        }
+    }
+
+    /* We just mark the handle as being destroyed.
+     * Deleting all the stuff will be deferred to the actual struct free. */
+    return UserDeleteObject(CurIcon->head.h, TYPE_CURSOR);
+}
+
+void
+FreeCurIconObject(
+    _In_ PVOID Object)
+{
+    PCURICON_OBJECT CurIcon = Object;
+    
+    if(!(CurIcon->CURSORF_flags & CURSORF_ACON))
+    {
+        HBITMAP bmpMask = CurIcon->hbmMask;
+        HBITMAP bmpColor = CurIcon->hbmColor;
+        HBITMAP bmpAlpha = CurIcon->hbmAlpha;
+
+        /* Delete bitmaps */
+        if (bmpMask)
+        {
+            GreSetObjectOwner(bmpMask, GDI_OBJ_HMGR_POWNED);
+            GreDeleteObject(bmpMask);
+            CurIcon->hbmMask = NULL;
+        }
+        if (bmpColor)
+        {
+            GreSetObjectOwner(bmpColor, GDI_OBJ_HMGR_POWNED);
+            GreDeleteObject(bmpColor);
+            CurIcon->hbmColor = NULL;
+        }
+        if (bmpAlpha)
+        {
+            GreSetObjectOwner(bmpAlpha, GDI_OBJ_HMGR_POWNED);
+            GreDeleteObject(bmpAlpha);
+            CurIcon->hbmAlpha = NULL;
+        }
+    }
+    else
+    {
+        PACON AniCurIcon = (PACON)CurIcon;
+        UINT i;
+
+        for(i = 0; i < AniCurIcon->cpcur; i++)
+            IntDestroyCurIconObject(AniCurIcon->aspcur[i]);
+        ExFreePoolWithTag(AniCurIcon->aspcur, USERTAG_CURSOR);
+    }
+
+    if (CurIcon->CURSORF_flags & CURSORF_LRSHARED)
+    {
+        if (!IS_INTRESOURCE(CurIcon->strName.Buffer))
+            ExFreePoolWithTag(CurIcon->strName.Buffer, TAG_STRING);
+        if (CurIcon->atomModName)
+            RtlDeleteAtomFromAtomTable(gAtomTable, CurIcon->atomModName);
+        CurIcon->strName.Buffer = NULL;
+        CurIcon->atomModName = 0;
     }

/* Finally free the thing */
@@ -635,7 +645,7 @@
 {
     BOOL ret;

-    TRACE("Enter NtUserDestroyCursorIcon\n");
+    TRACE("Enter NtUserDestroyCursorIcon (%p, %u)\n", hCurIcon, bForce);
     UserEnterExclusive();

if (!bForce)
@@ -842,7 +852,7 @@
     PCURICON_OBJECT pcurOld, pcurNew;
     HCURSOR hOldCursor = NULL;

-    TRACE("Enter NtUserSetCursor\n");
+    TRACE("Enter NtUserSetCursor: %p\n", hCursor);
     UserEnterExclusive();

if (hCursor)
@@ -863,6 +873,10 @@
     pcurOld = UserSetCursor(pcurNew, FALSE);
     if (pcurOld)
     {
+        hOldCursor = pcurOld->head.h;
+        /* See if it was destroyed in the meantime */
+        if (UserObjectInDestroy(hOldCursor))
+            hOldCursor = NULL;
         pcurOld->CURSORF_flags &= ~CURSORF_CURRENT;
         UserDereferenceObject(pcurOld);
     }