The SwapBuffers request requires that we trigger the swap at some point in the future. Sane drivers implement this by passing this request to something that will trigger a callback with the buffer pointers at the appropriate time.
The client can cause those buffers to be freed in X before the trigger occurs, most easily by quitting. This leads to a server crash in the driver when trying to do the swap. See http://bugs.freedesktop.org/show_bug.cgi?id=29065 for Radeon and https://bugs.freedesktop.org/show_bug.cgi?id=28080 for Intel. Signed-off-by: Christopher James Halse Rogers <[email protected]> --- hw/xfree86/dri2/dri2.c | 98 ++++++++++++++++++++++++++++++++++++++--------- hw/xfree86/dri2/dri2.h | 1 + 2 files changed, 80 insertions(+), 19 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index e4693d9..6da2e17 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -107,12 +107,41 @@ typedef struct _DRI2Screen { ConfigNotifyProcPtr ConfigNotify; } DRI2ScreenRec; +typedef struct _DRI2SwapCompleteDataRec { + DRI2BufferPtr src; + DRI2BufferPtr dest; + void * data; +} DRI2SwapCompleteDataRec, *DRI2SwapCompleteDataPtr; + static DRI2ScreenPtr DRI2GetScreen(ScreenPtr pScreen) { return dixLookupPrivate(&pScreen->devPrivates, dri2ScreenPrivateKey); } +static void +buffer_ref(DRI2BufferPtr buffer) +{ + buffer->refcnt++; +} + +static void +buffer_unref(DrawablePtr pDraw, DRI2BufferPtr buffer) +{ + DRI2ScreenPtr ds; + if (buffer->refcnt == 0) { + xf86DrvMsg(pDraw->pScreen->myNum, X_ERROR, + "[DRI2] Attempt to unreference already freed buffer ignored\n"); + return; + } + + buffer->refcnt--; + if (buffer->refcnt == 0) { + ds = DRI2GetScreen(pDraw->pScreen); + (*ds->DestroyBuffer)(pDraw, buffer); + } +} + static DRI2DrawablePtr DRI2GetDrawable(DrawablePtr pDraw) { @@ -261,7 +290,6 @@ DRI2CreateDrawable(ClientPtr client, DrawablePtr pDraw, XID id, static int DRI2DrawableGone(pointer p, XID id) { DRI2DrawablePtr pPriv = p; - DRI2ScreenPtr ds = pPriv->dri2_screen; DRI2DrawableRefPtr ref, next; WindowPtr pWin; PixmapPtr pPixmap; @@ -300,7 +328,7 @@ static int DRI2DrawableGone(pointer p, XID id) if (pPriv->buffers != NULL) { for (i = 0; i < pPriv->bufferCount; i++) - (*ds->DestroyBuffer)(pDraw, pPriv->buffers[i]); + buffer_unref(pDraw, pPriv->buffers[i]); free(pPriv->buffers); } @@ -341,6 +369,7 @@ allocate_or_reuse_buffer(DrawablePtr pDraw, DRI2ScreenPtr ds, || !dimensions_match || (pPriv->buffers[old_buf]->format != format)) { *buffer = (*ds->CreateBuffer)(pDraw, attachment, format); + buffer_ref(*buffer); pPriv->serialNumber = DRI2DrawableSerial(pDraw); return TRUE; @@ -355,13 +384,12 @@ static void update_dri2_drawable_buffers(DRI2DrawablePtr pPriv, DrawablePtr pDraw, DRI2BufferPtr *buffers, int *out_count, int *width, int *height) { - DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); int i; if (pPriv->buffers != NULL) { for (i = 0; i < pPriv->bufferCount; i++) { if (pPriv->buffers[i] != NULL) { - (*ds->DestroyBuffer)(pDraw, pPriv->buffers[i]); + buffer_unref(pDraw, pPriv->buffers[i]); } } @@ -498,7 +526,7 @@ err_out: for (i = 0; i < count; i++) { if (buffers[i] != NULL) - (*ds->DestroyBuffer)(pDraw, buffers[i]); + buffer_unref(pDraw, buffers[i]); } free(buffers); @@ -708,21 +736,31 @@ DRI2WakeClient(ClientPtr client, DrawablePtr pDraw, int frame, } } +static void +free_swap_complete_data (DrawablePtr pDraw, DRI2SwapCompleteDataPtr pSwapData) +{ + buffer_unref(pDraw, pSwapData->src); + buffer_unref(pDraw, pSwapData->dest); + free(pSwapData); +} + void DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, unsigned int tv_sec, unsigned int tv_usec, int type, DRI2SwapEventPtr swap_complete, void *swap_data) { - ScreenPtr pScreen = pDraw->pScreen; - DRI2DrawablePtr pPriv; - CARD64 ust = 0; - BoxRec box; - RegionRec region; + ScreenPtr pScreen = pDraw->pScreen; + DRI2DrawablePtr pPriv; + CARD64 ust = 0; + BoxRec box; + RegionRec region; + DRI2SwapCompleteDataPtr pSwapData = swap_data; pPriv = DRI2GetDrawable(pDraw); if (pPriv == NULL) { xf86DrvMsg(pScreen->myNum, X_ERROR, "[DRI2] %s: bad drawable\n", __func__); + free_swap_complete_data(pDraw, pSwapData); return; } @@ -739,12 +777,15 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, ust = ((CARD64)tv_sec * 1000000) + tv_usec; if (swap_complete) - swap_complete(client, swap_data, type, ust, frame, pPriv->swap_count); + swap_complete(client, pSwapData->data, type, ust, frame, + pPriv->swap_count); pPriv->last_swap_msc = frame; pPriv->last_swap_ust = ust; DRI2WakeClient(client, pDraw, frame, tv_sec, tv_usec); + + free_swap_complete_data(pDraw, pSwapData); } Bool @@ -771,12 +812,13 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, CARD64 divisor, CARD64 remainder, CARD64 *swap_target, DRI2SwapEventPtr func, void *data) { - ScreenPtr pScreen = pDraw->pScreen; - DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); - DRI2DrawablePtr pPriv; - DRI2BufferPtr pDestBuffer = NULL, pSrcBuffer = NULL; - int ret, i; - CARD64 ust, current_msc; + ScreenPtr pScreen = pDraw->pScreen; + DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); + DRI2DrawablePtr pPriv; + DRI2BufferPtr pDestBuffer = NULL, pSrcBuffer = NULL; + DRI2SwapCompleteDataPtr pSwapData; + int ret, i; + CARD64 ust, current_msc; pPriv = DRI2GetDrawable(pDraw); if (pPriv == NULL) { @@ -796,6 +838,23 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, "[DRI2] %s: drawable has no back or front?\n", __func__); return BadDrawable; } + + pSwapData = malloc(sizeof *pSwapData); + if (pSwapData == NULL) + return BadAlloc; + + /* + * Take a reference to the buffers we're exchanging. + * This ensures that these buffers will remain valid until the swap + * is completed. + * + * DRI2SwapComplete is required to unref these buffers. + */ + buffer_ref(pSrcBuffer); + buffer_ref(pDestBuffer); + pSwapData->src = pSrcBuffer; + pSwapData->dest = pDestBuffer; + pSwapData->data = data; /* Old DDX or no swap interval, just blit */ if (!ds->ScheduleSwap || !pPriv->swap_interval) { @@ -812,7 +871,7 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, (*ds->CopyRegion)(pDraw, ®ion, pDestBuffer, pSrcBuffer); DRI2SwapComplete(client, pDraw, target_msc, 0, 0, DRI2_BLIT_COMPLETE, - func, data); + func, pSwapData); return Success; } @@ -851,11 +910,12 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, pPriv->swapsPending++; ret = (*ds->ScheduleSwap)(client, pDraw, pDestBuffer, pSrcBuffer, - swap_target, divisor, remainder, func, data); + swap_target, divisor, remainder, func, pSwapData); if (!ret) { pPriv->swapsPending--; /* didn't schedule */ xf86DrvMsg(pScreen->myNum, X_ERROR, "[DRI2] %s: driver failed to schedule swap\n", __func__); + free_swap_complete_data(pDraw, pSwapData); return BadDrawable; } diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index fe0bf6c..12343fd 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -42,6 +42,7 @@ typedef struct { unsigned int pitch; unsigned int cpp; unsigned int flags; + unsigned int refcnt; unsigned int format; void *driverPrivate; } DRI2BufferRec, *DRI2BufferPtr; -- 1.7.2.3 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
