dix: Stop recycling scratch pixmaps

The minimal performance wins we gain by recycling pixmaps at this layer are
not worth the code complexity nor the interference with memory analysis
tools like malloc history, ASan, etc.

Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>

2 files changed, 12 insertions, 19 deletions

diff --git a/dix/pixmap.c b/dix/pixmap.c
index 0b01c4ee0..b7411df65 100644
--- a/dix/pixmap.c
+++ b/dix/pixmap.c
@@ -44,8 +44,13 @@ from The Open Group.
 #include "picturestr.h"
 #include "randrstr.h"
 /*
- *  Scratch pixmap management and device independent pixmap allocation
- *  function.
+ * Scratch pixmap APIs are provided for source and binary compatability.  In
+ * older versions, DIX would store a freed scratch pixmap for future use.  This
+ * optimization is not really that impactful on modern systems with decent
+ * system heap management and modern CPUs, and it interferes with memory
+ * analysis tools such as ASan, malloc history, etc.
+ *
+ * Now, these entry points just allocte/free pixmaps.
  */
 
 /* callable by ddx */
@@ -53,14 +58,7 @@ PixmapPtr
 GetScratchPixmapHeader(ScreenPtr pScreen, int width, int height, int depth,
                        int bitsPerPixel, int devKind, void *pPixData)
 {
-    PixmapPtr pPixmap = pScreen->pScratchPixmap;
-
-    if (pPixmap)
-        pScreen->pScratchPixmap = NULL;
-    else
-        /* width and height of 0 means don't allocate any pixmap data */
-        pPixmap = (*pScreen->CreatePixmap) (pScreen, 0, 0, depth, 0);
-
+    PixmapPtr pPixmap = (*pScreen->CreatePixmap) (pScreen, 0, 0, depth, 0);
     if (pPixmap) {
         if ((*pScreen->ModifyPixmapHeader) (pPixmap, width, height, depth,
                                             bitsPerPixel, devKind, pPixData))
@@ -76,12 +74,8 @@ FreeScratchPixmapHeader(PixmapPtr pPixmap)
 {
     if (pPixmap) {
         ScreenPtr pScreen = pPixmap->drawable.pScreen;
-
-        pPixmap->devPrivate.ptr = NULL; /* lest ddx chases bad ptr */
-        if (pScreen->pScratchPixmap)
-            (*pScreen->DestroyPixmap) (pPixmap);
-        else
-            pScreen->pScratchPixmap = pPixmap;
+        pPixmap->devPrivate.ptr = NULL; /* help catch/avoid heap-use-after-free */
+        (*pScreen->DestroyPixmap)(pPixmap);
     }
 }
 
@@ -94,7 +88,7 @@ CreateScratchPixmapsForScreen(ScreenPtr pScreen)
     pScreen->totalPixmapSize =
         BitmapBytePad(pixmap_size * 8);
 
-    /* let it be created on first use */
+    /* NULL this out as it is no longer used */
     pScreen->pScratchPixmap = NULL;
     return TRUE;
 }
@@ -102,7 +96,6 @@ CreateScratchPixmapsForScreen(ScreenPtr pScreen)
 void
 FreeScratchPixmapsForScreen(ScreenPtr pScreen)
 {
-    FreeScratchPixmapHeader(pScreen->pScratchPixmap);
 }
 
 /* callable by ddx */
diff --git a/include/scrnintstr.h b/include/scrnintstr.h
index 24ca423f2..6d09e8019 100644
--- a/include/scrnintstr.h
+++ b/include/scrnintstr.h
@@ -604,7 +604,7 @@ typedef struct _Screen {
     SetScreenPixmapProcPtr SetScreenPixmap;
     NameWindowPixmapProcPtr NameWindowPixmap;
 
-    PixmapPtr pScratchPixmap;   /* scratch pixmap "pool" */
+    PixmapPtr pScratchPixmap;   /* scratch pixmap "pool" (unused / NULL in modern servers) */
 
     unsigned int totalPixmapSize;