dri2: hold extra buffer ref during swap

It is possible that the client detaches while we are waiting for the
page_flip event.  Use ref counting on dri2 buffer to avoid freeing
a buffer before the page_flip event is processed.

1 files changed, 65 insertions, 20 deletions

diff --git a/src/omap_dri2.c b/src/omap_dri2.c
index d8e78b7..3e5878f 100644
--- a/src/omap_dri2.c
+++ b/src/omap_dri2.c
@@ -61,6 +61,13 @@ typedef struct {
 	 */
 	uint32_t fb_id;
 
+	/**
+	 * The DRI2 buffers are reference counted to avoid crashyness when the
+	 * client detaches a dri2 drawable while we are still waiting for a
+	 * page_flip event.
+	 */
+	int refcnt;
+
 } OMAPDRI2BufferRec, *OMAPDRI2BufferPtr;
 
 #define OMAPBUF(p)	((OMAPDRI2BufferPtr)(p))
@@ -162,6 +169,8 @@ OMAPDRI2CreateBuffer(DrawablePtr pDraw, unsigned int attachment,
 
 			pScreen->DestroyPixmap(pNewPix);
 		}
+
+		pPixmap->refcnt++;
 	} else {
 		pPixmap = createpix(pDraw);
 	}
@@ -172,6 +181,8 @@ OMAPDRI2CreateBuffer(DrawablePtr pDraw, unsigned int attachment,
 	DRIBUF(buf)->pitch = exaGetPixmapPitch(pPixmap);
 	DRIBUF(buf)->cpp = pPixmap->drawable.bitsPerPixel / 8;
 	DRIBUF(buf)->format = format;
+	buf->refcnt = 1;
+	buf->pPixmap = pPixmap;
 
 	ret = omap_bo_get_name(bo, &DRIBUF(buf)->name);
 	if (ret) {
@@ -180,10 +191,6 @@ OMAPDRI2CreateBuffer(DrawablePtr pDraw, unsigned int attachment,
 		return NULL;
 	}
 
-	if (attachment != DRI2BufferFrontLeft) {
-		buf->pPixmap = pPixmap;
-	}
-
 	/* Q: how to know across OMAP generations what formats that the display
 	 * can support directly?
 	 * A: attempt to create a drm_framebuffer, and if that fails then the
@@ -213,10 +220,16 @@ OMAPDRI2CreateBuffer(DrawablePtr pDraw, unsigned int attachment,
 static void
 OMAPDRI2DestroyBuffer(DrawablePtr pDraw, DRI2BufferPtr buffer)
 {
-	ScreenPtr pScreen = pDraw->pScreen;
+	OMAPDRI2BufferPtr buf = OMAPBUF(buffer);
+	/* Note: pDraw may already be deleted, so use the pPixmap here
+	 * instead (since it is at least refcntd)
+	 */
+	ScreenPtr pScreen = buf->pPixmap->drawable.pScreen;
 	ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
 	OMAPPtr pOMAP = OMAPPTR(pScrn);
-	OMAPDRI2BufferPtr buf = OMAPBUF(buffer);
+
+	if (--buf->refcnt > 0)
+		return;
 
 	DEBUG_MSG("pDraw=%p, buffer=%p", pDraw, buffer);
 
@@ -224,15 +237,18 @@ OMAPDRI2DestroyBuffer(DrawablePtr pDraw, DRI2BufferPtr buffer)
 		drmModeRmFB(pOMAP->drmFD, buf->fb_id);
 	}
 
-	if (DRIBUF(buf)->attachment == DRI2BufferFrontLeft) {
-		// XXX don't destroy.. I think..
-	} else {
-		pScreen->DestroyPixmap(buf->pPixmap);
-	}
+	pScreen->DestroyPixmap(buf->pPixmap);
 
 	free(buf);
 }
 
+static void
+OMAPDRI2ReferenceBuffer(DRI2BufferPtr buffer)
+{
+	OMAPDRI2BufferPtr buf = OMAPBUF(buffer);
+	buf->refcnt++;
+}
+
 /**
  *
  */
@@ -313,27 +329,49 @@ OMAPDRI2GetMSC(DrawablePtr pDraw, CARD64 *ust, CARD64 *msc)
 struct _OMAPDRISwapCmd {
 	int type;
 	ClientPtr client;
-	DrawablePtr pDraw;
+	ScreenPtr pScreen;
+	/* Note: store drawable ID, rather than drawable.  It's possible that
+	 * the drawable can be destroyed while we wait for page flip event:
+	 */
+	XID draw_id;
 	DRI2BufferPtr pDstBuffer;
 	DRI2BufferPtr pSrcBuffer;
 	DRI2SwapEventPtr func;
 	void *data;
 };
 
+static const char *swap_names[] = {
+		[DRI2_EXCHANGE_COMPLETE] = "exchange",
+		[DRI2_BLIT_COMPLETE] = "blit",
+		[DRI2_FLIP_COMPLETE] = "flip,"
+};
+
 void
 OMAPDRI2SwapComplete(OMAPDRISwapCmd *cmd)
 {
-	ScreenPtr pScreen = cmd->pDraw->pScreen;
+	ScreenPtr pScreen = cmd->pScreen;
 	ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
+	DrawablePtr pDraw = NULL;
+	int status;
+
+	DEBUG_MSG("%s complete: %d -> %d", swap_names[cmd->type],
+			cmd->pSrcBuffer->attachment, cmd->pDstBuffer->attachment);
 
-	DEBUG_MSG("%d -> %d", cmd->pSrcBuffer->attachment,
-			cmd->pDstBuffer->attachment);
+	status = dixLookupDrawable(&pDraw, cmd->draw_id, serverClient,
+			M_ANY, DixWriteAccess);
 
-	if (cmd->type != DRI2_BLIT_COMPLETE)
-		exchangebufs(cmd->pDraw, cmd->pSrcBuffer, cmd->pDstBuffer);
+	if (status == Success) {
+		if (cmd->type != DRI2_BLIT_COMPLETE)
+			exchangebufs(pDraw, cmd->pSrcBuffer, cmd->pDstBuffer);
 
-	DRI2SwapComplete(cmd->client, cmd->pDraw, 0, 0, 0,
-			cmd->type, cmd->func, cmd->data);
+		DRI2SwapComplete(cmd->client, pDraw, 0, 0, 0, cmd->type,
+				cmd->func, cmd->data);
+	}
+
+	/* drop extra refcnt we obtained prior to swap:
+	 */
+	OMAPDRI2DestroyBuffer(pDraw, cmd->pSrcBuffer);
+	OMAPDRI2DestroyBuffer(pDraw, cmd->pDstBuffer);
 
 	free(cmd);
 }
@@ -363,7 +401,8 @@ OMAPDRI2ScheduleSwap(ClientPtr client, DrawablePtr pDraw,
 	OMAPDRISwapCmd *cmd = calloc(1, sizeof(*cmd));
 
 	cmd->client = client;
-	cmd->pDraw = pDraw;
+	cmd->pScreen = pScreen;
+	cmd->draw_id = pDraw->id;
 	cmd->pSrcBuffer = pSrcBuffer;
 	cmd->pDstBuffer = pDstBuffer;
 	cmd->func = func;
@@ -371,6 +410,12 @@ OMAPDRI2ScheduleSwap(ClientPtr client, DrawablePtr pDraw,
 
 	DEBUG_MSG("%d -> %d", pSrcBuffer->attachment, pDstBuffer->attachment);
 
+	/* obtain extra ref on buffers to avoid them going away while we await
+	 * the page flip event:
+	 */
+	OMAPDRI2ReferenceBuffer(pSrcBuffer);
+	OMAPDRI2ReferenceBuffer(pDstBuffer);
+
 	if (src->fb_id && dst->fb_id) {
 		DEBUG_MSG("can flip:  %d -> %d", src->fb_id, dst->fb_id);
 		cmd->type = DRI2_FLIP_COMPLETE;