[ros-diffs] [tkreuzer] 50827: [WIN32K] NtGdiSelectBitmap. Don't exclusively lock the old bitmap. This could cause pool corruptions, when the share count reachted 0, the object was deleted and unlocked after th...

Author: tkreuzer
Date: Sat Feb 19 23:44:36 2011
New Revision: 50827

URL: http://svn.reactos.org/svn/reactos?rev=50827&view=rev
Log:
[WIN32K]
NtGdiSelectBitmap. Don't exclusively lock the old bitmap. This could cause pool
corruptions, when the share count reachted 0, the object was deleted and unlocked after
that. Use InterlockedCompareExchange to check and exchange the new bitmaps dc. This is
commented out, because it causes a lot of drawing problems. Do the referencing manually
instead of calling DC_vSelectSurface. Use a shared reference instead of an exclusive lock
for the new bitmap. add code for proper handling of DEFAULT_BITMAP, currently hacked due
to restrictions in other parts of win32k. Fixes bug 5498 and probably a lot of other
problems.

Modified:
    trunk/reactos/subsystems/win32/win32k/objects/dcobjs.c

Modified: trunk/reactos/subsystems/win32/win32k/objects/dcobjs.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/ob…
==============================================================================
--- trunk/reactos/subsystems/win32/win32k/objects/dcobjs.c [iso-8859-1] (original)
+++ trunk/reactos/subsystems/win32/win32k/objects/dcobjs.c [iso-8859-1] Sat Feb 19
23:44:36 2011
@@ -251,89 +251,119 @@
 HBITMAP
 APIENTRY
 NtGdiSelectBitmap(
-    IN HDC hDC,
-    IN HBITMAP hBmp)
-{
-    PDC pDC;
+    IN HDC hdc,
+    IN HBITMAP hbmp)
+{
+    PDC pdc;
     PDC_ATTR pdcattr;
-    HBITMAP hOrgBmp;
-    PSURFACE psurfBmp, psurfOld;
+    HBITMAP hbmpOld;
+    PSURFACE psurfNew, psurfOld;
     HRGN hVisRgn;
-
-    if (hDC == NULL || hBmp == NULL) return NULL;
-
-    pDC = DC_LockDc(hDC);
-    if (!pDC)
-    {
-        return NULL;
-    }
-    pdcattr = pDC->pdcattr;
-
-    /* must be memory dc to select bitmap */
-    if (pDC->dctype != DC_TYPE_MEMORY)
-    {
-        DC_UnlockDc(pDC);
-        return NULL;
-    }
-
-    psurfBmp = SURFACE_LockSurface(hBmp);
-    if (!psurfBmp)
-    {
-        DC_UnlockDc(pDC);
-        return NULL;
-    }
-
-    /* Get the handle for the old bitmap */
-    ASSERT(pDC->dclevel.pSurface);
-    hOrgBmp = pDC->dclevel.pSurface->BaseObject.hHmgr;
-
-	/* Lock it, to be sure while we mess with it*/
-	psurfOld = SURFACE_LockSurface(hOrgBmp);
-
-	/* Reset hdc, this surface isn't selected anymore */
-	psurfOld->hdc = NULL;
-
-    /* Release the old bitmap, reference the new */
-    DC_vSelectSurface(pDC, psurfBmp);
-
-	/* And unlock it, now we're done */
-	SURFACE_UnlockSurface(psurfOld);
-
-    // If Info DC this is zero and pSurface is moved to DC->pSurfInfo.
-    psurfBmp->hdc = hDC;
-
+    SIZEL sizlBitmap = {1, 1};
+    ASSERT_NOGDILOCKS();
+
+    /* Verify parameters */
+    if (hdc == NULL || hbmp == NULL) return NULL;
+
+    /* First lock the DC */
+    pdc = DC_LockDc(hdc);
+    if (!pdc)
+    {
+        return NULL;
+    }
+    pdcattr = pdc->pdcattr;
+
+    /* Must be a memory dc to select a bitmap */
+    if (pdc->dctype != DC_TYPE_MEMORY)
+    {
+        DC_UnlockDc(pdc);
+        return NULL;
+    }
+
+    /* Save the old bitmap */
+    psurfOld = pdc->dclevel.pSurface;
+
+    /* Check if the default bitmap was passed */
+    if (hbmp == StockObjects[DEFAULT_BITMAP])
+    {
+        psurfNew = NULL;
+
+        // HACK
+        psurfNew = SURFACE_ShareLockSurface(hbmp);
+    }
+    else
+    {
+        /* Reference the new bitmap and check if it's valid */
+        psurfNew = SURFACE_ShareLockSurface(hbmp);
+        if (!psurfNew)
+        {
+            DC_UnlockDc(pdc);
+            return NULL;
+        }
+#if 0 // FIXME: bug bug, causes problems
+        /* Set the bitmp's hdc */
+        if (InterlockedCompareExchangePointer((PVOID*)&psurfNew->hdc, hdc, 0))
+        {
+            /* The bitmap is already selected, fail */
+            SURFACE_ShareUnlockSurface(psurfNew);
+            DC_UnlockDc(pdc);
+            return NULL;
+        }
+#endif
+        /* Get the bitmap size */
+        sizlBitmap = psurfNew->SurfObj.sizlBitmap;
+
+        /* Check if the bitmap is a dibsection */
+        if(psurfNew->hSecure)
+        {
+            /* Set DIBSECTION attribute */
+            pdcattr->ulDirty_ |= DC_DIBSECTION;
+        }
+        else
+        {
+            pdcattr->ulDirty_ &= ~DC_DIBSECTION;
+        }
+    }
+
+    /* Select the new bitmap */
+    pdc->dclevel.pSurface = psurfNew;
+
+    /* Check if there was a bitmap selected before */
+    if (psurfOld)
+    {
+        hbmpOld = psurfOld->BaseObject.hHmgr;
+
+        /* Reset hdc of old bitmap, this surface isn't selected anymore */
+        psurfOld->hdc = NULL;
+
+        /* Release the old bitmap */
+        SURFACE_ShareUnlockSurface(psurfOld);
+    }
+    else
+    {
+        /* Return default bitmap */
+        hbmpOld = StockObjects[DEFAULT_BITMAP];
+    }
+
+    /* Mark the dc brushes invalid */
+    pdcattr->ulDirty_ |= DIRTY_FILL | DIRTY_LINE;
+
+    /* Unlock the DC */
+    DC_UnlockDc(pdc);
 
     /* FIXME; improve by using a region without a handle and selecting it */
     hVisRgn = IntSysCreateRectRgn( 0,
                                    0,
-                                   psurfBmp->SurfObj.sizlBitmap.cx,
-                                   psurfBmp->SurfObj.sizlBitmap.cy);
-
-    if(psurfBmp->hSecure)
-    {
-        /* Set DIBSECTION attribute */
-        pdcattr->ulDirty_ |= DC_DIBSECTION;
-    }
-    else
-    {
-        pdcattr->ulDirty_ &= ~DC_DIBSECTION;
-    }
-
-    /* Release the exclusive lock */
-    SURFACE_UnlockSurface(psurfBmp);
-
-    /* Mark the brushes invalid */
-    pdcattr->ulDirty_ |= DIRTY_FILL | DIRTY_LINE;
-
-    DC_UnlockDc(pDC);
-
+                                   sizlBitmap.cx,
+                                   sizlBitmap.cy);
     if (hVisRgn)
     {
-        GdiSelectVisRgn(hDC, hVisRgn);
+        GdiSelectVisRgn(hdc, hVisRgn);
         REGION_FreeRgnByHandle(hVisRgn);
     }
 
-    return hOrgBmp;
+    /* Return the old bitmp handle */
+    return hbmpOld;
 }