[PATCH] DRI2 resource management fixes.
Michel Dänzer
michel at daenzer.net
Sun Aug 2 07:59:54 PDT 2009
From: Michel Dänzer <daenzer at vmware.com>
* Add new variants of DRI2CreateDrawable, DRI2DestroyDrawable and
DRI2CopyRegion which take a ClientPtr and an XID instead of a DrawablePtr.
Use these in dri2ext.c always and in glxdri2.c for windows (not all pixmaps
have an associated XID, and we can increase their reference count to prevent
them from disappearing beneath us) to avoid derefencing a DrawablePtr for a
window which has been destroyed. Fixes crashes when quitting MacSlow's
rgba-glx demo.
* Move DRI2 drawable resource tracking from dri2ext.c to dri2.c, and add screen
DestroyPixmap/Window hooks to prevent DRI2 drawable leaks under any
circumstances.
---
glx/glxcmds.c | 26 ++++-----
glx/glxdrawable.h | 1 +
glx/glxdri2.c | 39 ++++++++----
hw/xfree86/dri2/dri2.c | 145 +++++++++++++++++++++++++++++++++++++++++++--
hw/xfree86/dri2/dri2.h | 8 +++
hw/xfree86/dri2/dri2ext.c | 32 ++--------
6 files changed, 191 insertions(+), 60 deletions(-)
diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index f5632d1..5112303 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -1085,7 +1085,8 @@ __glXDrawableInit(__GLXdrawable *drawable,
__GLXscreen *screen, DrawablePtr pDraw, int type,
XID drawId, __GLXconfig *config)
{
- drawable->pDraw = pDraw;
+ drawable->pScreen = pDraw->pScreen;
+ drawable->pDraw = pDraw->type == DRAWABLE_PIXMAP ? pDraw : NULL;
drawable->type = type;
drawable->drawId = drawId;
drawable->config = config;
@@ -1097,19 +1098,14 @@ __glXDrawableInit(__GLXdrawable *drawable,
void
__glXDrawableRelease(__GLXdrawable *drawable)
{
- ScreenPtr pScreen = drawable->pDraw->pScreen;
-
- switch (drawable->type) {
- case GLX_DRAWABLE_PIXMAP:
- case GLX_DRAWABLE_PBUFFER:
- (*pScreen->DestroyPixmap)((PixmapPtr) drawable->pDraw);
- break;
- }
+ /* We only store the DrawablePtr for pixmaps */
+ if (drawable->pDraw)
+ (*drawable->pScreen->DestroyPixmap)((PixmapPtr) drawable->pDraw);
}
static int
DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *config,
- DrawablePtr pDraw, XID glxDrawableId, int type)
+ DrawablePtr pDraw, XID drawableId, XID glxDrawableId, int type)
{
__GLXdrawable *pGlxDraw;
@@ -1119,7 +1115,7 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *conf
return BadMatch;
pGlxDraw = pGlxScreen->createDrawable(pGlxScreen, pDraw, type,
- glxDrawableId, config);
+ drawableId, config);
if (pGlxDraw == NULL)
return BadAlloc;
@@ -1148,7 +1144,7 @@ DoCreateGLXPixmap(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *config
return BadPixmap;
}
- err = DoCreateGLXDrawable(client, pGlxScreen, config, pDraw,
+ err = DoCreateGLXDrawable(client, pGlxScreen, config, pDraw, drawableId,
glxDrawableId, GLX_DRAWABLE_PIXMAP);
if (err == Success)
@@ -1305,7 +1301,7 @@ DoCreatePbuffer(ClientPtr client, int screenNum, XID fbconfigId,
__glXleaveServer(GL_FALSE);
return DoCreateGLXDrawable(client, pGlxScreen, config, &pPixmap->drawable,
- glxDrawableId, GLX_DRAWABLE_PBUFFER);
+ glxDrawableId, glxDrawableId, GLX_DRAWABLE_PBUFFER);
}
int __glXDisp_CreatePbuffer(__GLXclientState *cl, GLbyte *pc)
@@ -1425,8 +1421,8 @@ int __glXDisp_CreateWindow(__GLXclientState *cl, GLbyte *pc)
if (!validGlxFBConfigForWindow(client, config, pDraw, &err))
return err;
- return DoCreateGLXDrawable(client, pGlxScreen, config,
- pDraw, req->glxwindow, GLX_DRAWABLE_WINDOW);
+ return DoCreateGLXDrawable(client, pGlxScreen, config, pDraw,
+ req->window, req->glxwindow, GLX_DRAWABLE_WINDOW);
}
int __glXDisp_DestroyWindow(__GLXclientState *cl, GLbyte *pc)
diff --git a/glx/glxdrawable.h b/glx/glxdrawable.h
index 3f165ed..922eefd 100644
--- a/glx/glxdrawable.h
+++ b/glx/glxdrawable.h
@@ -51,6 +51,7 @@ struct __GLXdrawable {
void (*waitX)(__GLXdrawable *);
void (*waitGL)(__GLXdrawable *);
+ ScreenPtr pScreen;
DrawablePtr pDraw;
XID drawId;
diff --git a/glx/glxdri2.c b/glx/glxdri2.c
index ed7fb4c..e8fde0d 100644
--- a/glx/glxdri2.c
+++ b/glx/glxdri2.c
@@ -104,10 +104,10 @@ __glXDRIdrawableDestroy(__GLXdrawable *drawable)
(*core->destroyDrawable)(private->driDrawable);
- /* If the X window was destroyed, the dri DestroyWindow hook will
- * aready have taken care of this, so only call if pDraw isn't NULL. */
- if (drawable->pDraw != NULL)
+ if (drawable->pDraw)
DRI2DestroyDrawable(drawable->pDraw);
+ else
+ DRI2DestroyDrawableWithId(__pGlxClient, drawable->drawId);
__glXDrawableRelease(drawable);
@@ -126,10 +126,14 @@ __glXDRIdrawableCopySubBuffer(__GLXdrawable *drawable,
box.y1 = private->height - y - h;
box.x2 = x + w;
box.y2 = private->height - y;
- REGION_INIT(drawable->pDraw->pScreen, ®ion, &box, 0);
+ REGION_INIT(drawable->pScreen, ®ion, &box, 0);
- DRI2CopyRegion(drawable->pDraw, ®ion,
- DRI2BufferFrontLeft, DRI2BufferBackLeft);
+ if (drawable->pDraw)
+ DRI2CopyRegion(drawable->pDraw, ®ion,
+ DRI2BufferFrontLeft, DRI2BufferBackLeft);
+ else
+ DRI2CopyRegionWithId(__pGlxClient, drawable->drawId, ®ion,
+ DRI2BufferFrontLeft, DRI2BufferBackLeft);
}
static GLboolean
@@ -154,10 +158,14 @@ __glXDRIdrawableWaitX(__GLXdrawable *drawable)
box.y1 = 0;
box.x2 = private->width;
box.y2 = private->height;
- REGION_INIT(drawable->pDraw->pScreen, ®ion, &box, 0);
+ REGION_INIT(drawable->pScreen, ®ion, &box, 0);
- DRI2CopyRegion(drawable->pDraw, ®ion,
- DRI2BufferFakeFrontLeft, DRI2BufferFrontLeft);
+ if (drawable->pDraw)
+ DRI2CopyRegion(drawable->pDraw, ®ion,
+ DRI2BufferFakeFrontLeft, DRI2BufferFrontLeft);
+ else
+ DRI2CopyRegionWithId(__pGlxClient, drawable->drawId, ®ion,
+ DRI2BufferFakeFrontLeft, DRI2BufferFrontLeft);
}
static void
@@ -171,10 +179,14 @@ __glXDRIdrawableWaitGL(__GLXdrawable *drawable)
box.y1 = 0;
box.x2 = private->width;
box.y2 = private->height;
- REGION_INIT(drawable->pDraw->pScreen, ®ion, &box, 0);
+ REGION_INIT(drawable->pScreen, ®ion, &box, 0);
- DRI2CopyRegion(drawable->pDraw, ®ion,
- DRI2BufferFrontLeft, DRI2BufferFakeFrontLeft);
+ if (drawable->pDraw)
+ DRI2CopyRegion(drawable->pDraw, ®ion,
+ DRI2BufferFrontLeft, DRI2BufferFakeFrontLeft);
+ else
+ DRI2CopyRegionWithId(__pGlxClient, drawable->drawId, ®ion,
+ DRI2BufferFrontLeft, DRI2BufferFakeFrontLeft);
}
static int
@@ -387,7 +399,8 @@ __glXDRIscreenCreateDrawable(__GLXscreen *screen,
private->base.waitGL = __glXDRIdrawableWaitGL;
private->base.waitX = __glXDRIdrawableWaitX;
- if (DRI2CreateDrawable(pDraw)) {
+ if (private->base.pDraw ? DRI2CreateDrawable(private->base.pDraw) :
+ DRI2CreateDrawableWithId(__pGlxClient, private->base.drawId)) {
xfree(private);
return NULL;
}
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 8795cd1..d054b8a 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -42,6 +42,18 @@
#include "xf86.h"
+extern RESTYPE dri2DrawableRes;
+
+static Bool
+validDrawable(ClientPtr client, XID drawable, DrawablePtr *pDrawable, int *status)
+{
+ *status = dixLookupDrawable(pDrawable, drawable, client, 0, DixReadAccess);
+ if (*status != Success)
+ return FALSE;
+
+ return TRUE;
+}
+
static int dri2ScreenPrivateKeyIndex;
static DevPrivateKey dri2ScreenPrivateKey = &dri2ScreenPrivateKeyIndex;
static int dri2WindowPrivateKeyIndex;
@@ -68,6 +80,8 @@ typedef struct _DRI2Screen {
DRI2DestroyBufferProcPtr DestroyBuffer;
DRI2CopyRegionProcPtr CopyRegion;
+ DestroyPixmapProcPtr DestroyPixmap;
+ DestroyWindowProcPtr DestroyWindow;
HandleExposuresProcPtr HandleExposures;
} DRI2ScreenRec, *DRI2ScreenPtr;
@@ -133,6 +147,27 @@ DRI2CreateDrawable(DrawablePtr pDraw)
return Success;
}
+int
+DRI2CreateDrawableWithId(ClientPtr client, XID xDrawable)
+{
+ DrawablePtr pDraw;
+ int status;
+
+ if (!validDrawable(client, xDrawable, &pDraw, &status))
+ return status;
+
+ status = DRI2CreateDrawable(pDraw);
+ if (status != Success)
+ return status;
+
+ if (!AddResource(xDrawable, dri2DrawableRes, client)) {
+ DRI2DestroyDrawable(pDraw);
+ return BadAlloc;
+ }
+
+ return Success;
+}
+
static int
find_attachment(DRI2DrawablePtr pPriv, unsigned attachment)
{
@@ -337,8 +372,21 @@ DRI2CopyRegion(DrawablePtr pDraw, RegionPtr pRegion,
return Success;
}
-void
-DRI2DestroyDrawable(DrawablePtr pDraw)
+int
+DRI2CopyRegionWithId(ClientPtr client, XID xDrawable, RegionPtr pRegion,
+ unsigned int dest, unsigned int src)
+{
+ DrawablePtr pDraw;
+ int status;
+
+ if (!validDrawable(client, xDrawable, &pDraw, &status))
+ return status;
+
+ return DRI2CopyRegion(pDraw, pRegion, dest, src);
+}
+
+static void
+DRI2DoDestroyDrawable(DrawablePtr pDraw)
{
DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen);
DRI2DrawablePtr pPriv;
@@ -349,10 +397,6 @@ DRI2DestroyDrawable(DrawablePtr pDraw)
if (pPriv == NULL)
return;
- pPriv->refCount--;
- if (pPriv->refCount > 0)
- return;
-
if (pPriv->buffers != NULL) {
int i;
@@ -376,6 +420,87 @@ DRI2DestroyDrawable(DrawablePtr pDraw)
}
}
+void
+DRI2DestroyDrawable(DrawablePtr pDraw)
+{
+ DRI2DrawablePtr pPriv;
+
+ pPriv = DRI2GetDrawable(pDraw);
+ if (pPriv == NULL)
+ return;
+
+ pPriv->refCount--;
+ if (pPriv->refCount > 0)
+ return;
+
+ DRI2DoDestroyDrawable(pDraw);
+}
+
+int
+DRI2DestroyDrawableWithId(ClientPtr client, XID xDrawable)
+{
+ DrawablePtr pDraw;
+ int status;
+
+ if (!validDrawable(client, xDrawable, &pDraw, &status))
+ return status;
+
+ DRI2DestroyDrawable(pDraw);
+
+ return Success;
+}
+
+static Bool
+DRI2DestroyPixmap (PixmapPtr pPixmap)
+{
+ ScreenPtr pScreen = pPixmap->drawable.pScreen;
+ DRI2ScreenPtr ds = DRI2GetScreen(pScreen);
+ Bool retval = TRUE;
+
+ if (pPixmap->refcnt == 1)
+ DRI2DoDestroyDrawable(&pPixmap->drawable);
+
+ /* call lower wrapped functions */
+ if (ds->DestroyPixmap) {
+ /* unwrap */
+ pScreen->DestroyPixmap = ds->DestroyPixmap;
+
+ /* call lower layers */
+ retval = (*pScreen->DestroyPixmap)(pPixmap);
+
+ /* rewrap */
+ ds->DestroyPixmap = pScreen->DestroyPixmap;
+ pScreen->DestroyPixmap = DRI2DestroyPixmap;
+ }
+
+ return retval;
+}
+
+static Bool
+DRI2DestroyWindow(WindowPtr pWin)
+{
+ ScreenPtr pScreen = pWin->drawable.pScreen;
+ DRI2ScreenPtr ds = DRI2GetScreen(pScreen);
+ Bool retval = TRUE;
+
+ DRI2DoDestroyDrawable(&pWin->drawable);
+
+ /* call lower wrapped functions */
+ if (ds->DestroyWindow) {
+ /* unwrap */
+ pScreen->DestroyWindow = ds->DestroyWindow;
+
+ /* call lower layers */
+ retval = (*pScreen->DestroyWindow)(pWin);
+
+ /* rewrap */
+ ds->DestroyWindow = pScreen->DestroyWindow;
+ pScreen->DestroyWindow = DRI2DestroyWindow;
+ }
+
+ return retval;
+}
+
Bool
DRI2Connect(ScreenPtr pScreen, unsigned int driverType, int *fd,
const char **driverName, const char **deviceName)
@@ -426,6 +551,11 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
ds->DestroyBuffer = info->DestroyBuffer;
ds->CopyRegion = info->CopyRegion;
+ ds->DestroyPixmap = pScreen->DestroyPixmap;
+ pScreen->DestroyPixmap = DRI2DestroyPixmap;
+ ds->DestroyWindow = pScreen->DestroyWindow;
+ pScreen->DestroyWindow = DRI2DestroyWindow;
+
dixSetPrivate(&pScreen->devPrivates, dri2ScreenPrivateKey, ds);
xf86DrvMsg(pScreen->myNum, X_INFO, "[DRI2] Setup complete\n");
@@ -438,6 +568,9 @@ DRI2CloseScreen(ScreenPtr pScreen)
{
DRI2ScreenPtr ds = DRI2GetScreen(pScreen);
+ pScreen->DestroyPixmap = ds->DestroyPixmap;
+ pScreen->DestroyWindow = ds->DestroyWindow;
+
xfree(ds);
dixSetPrivate(&pScreen->devPrivates, dri2ScreenPrivateKey, NULL);
}
diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
index 175471a..cc4bbcb 100644
--- a/hw/xfree86/dri2/dri2.h
+++ b/hw/xfree86/dri2/dri2.h
@@ -100,8 +100,10 @@ extern _X_EXPORT Bool DRI2Connect(ScreenPtr pScreen,
extern _X_EXPORT Bool DRI2Authenticate(ScreenPtr pScreen, drm_magic_t magic);
extern _X_EXPORT int DRI2CreateDrawable(DrawablePtr pDraw);
+extern _X_EXPORT int DRI2CreateDrawableWithId(ClientPtr client, XID drawable);
extern _X_EXPORT void DRI2DestroyDrawable(DrawablePtr pDraw);
+extern _X_EXPORT int DRI2DestroyDrawableWithId(ClientPtr client, XID drawable);
extern _X_EXPORT DRI2BufferPtr *DRI2GetBuffers(DrawablePtr pDraw,
int *width,
@@ -115,6 +117,12 @@ extern _X_EXPORT int DRI2CopyRegion(DrawablePtr pDraw,
unsigned int dest,
unsigned int src);
+extern _X_EXPORT int DRI2CopyRegionWithId(ClientPtr client,
+ XID xDrawable,
+ RegionPtr pRegion,
+ unsigned int dest,
+ unsigned int src);
+
/**
* Determine the major and minor version of the DRI2 extension.
*
diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
index 029dce8..fe5de7d 100644
--- a/hw/xfree86/dri2/dri2ext.c
+++ b/hw/xfree86/dri2/dri2ext.c
@@ -50,7 +50,7 @@
#include "xf86Module.h"
static ExtensionEntry *dri2Extension;
-static RESTYPE dri2DrawableRes;
+RESTYPE dri2DrawableRes;
static Bool
validDrawable(ClientPtr client, XID drawable,
@@ -156,23 +156,14 @@ static int
ProcDRI2CreateDrawable(ClientPtr client)
{
REQUEST(xDRI2CreateDrawableReq);
- DrawablePtr pDrawable;
int status;
REQUEST_SIZE_MATCH(xDRI2CreateDrawableReq);
- if (!validDrawable(client, stuff->drawable, &pDrawable, &status))
- return status;
-
- status = DRI2CreateDrawable(pDrawable);
+ status = DRI2CreateDrawableWithId(client, stuff->drawable);
if (status != Success)
return status;
- if (!AddResource(stuff->drawable, dri2DrawableRes, pDrawable)) {
- DRI2DestroyDrawable(pDrawable);
- return BadAlloc;
- }
-
return client->noClientException;
}
@@ -180,16 +171,10 @@ static int
ProcDRI2DestroyDrawable(ClientPtr client)
{
REQUEST(xDRI2DestroyDrawableReq);
- DrawablePtr pDrawable;
- int status;
REQUEST_SIZE_MATCH(xDRI2DestroyDrawableReq);
- if (!validDrawable(client, stuff->drawable, &pDrawable, &status))
- return status;
-
- FreeResourceByType(stuff->drawable, dri2DrawableRes, FALSE);
- return client->noClientException;
+ return DRI2DestroyDrawableWithId(client, stuff->drawable);
}
@@ -290,18 +275,15 @@ ProcDRI2CopyRegion(ClientPtr client)
{
REQUEST(xDRI2CopyRegionReq);
xDRI2CopyRegionReply rep;
- DrawablePtr pDrawable;
int status;
RegionPtr pRegion;
REQUEST_SIZE_MATCH(xDRI2CopyRegionReq);
- if (!validDrawable(client, stuff->drawable, &pDrawable, &status))
- return status;
-
VERIFY_REGION(pRegion, stuff->region, client, DixReadAccess);
- status = DRI2CopyRegion(pDrawable, pRegion, stuff->dest, stuff->src);
+ status = DRI2CopyRegionWithId(client, stuff->drawable, pRegion, stuff->dest,
+ stuff->src);
if (status != Success)
return status;
@@ -398,9 +380,7 @@ SProcDRI2Dispatch (ClientPtr client)
static int DRI2DrawableGone(pointer p, XID id)
{
- DrawablePtr pDrawable = p;
-
- DRI2DestroyDrawable(pDrawable);
+ DRI2DestroyDrawableWithId(p, id);
return Success;
}
--
1.6.3.3
More information about the xorg-devel
mailing list