From: Christopher James Halse Rogers <christopher.halse.rog...@canonical.com>
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 <christopher.halse.rog...@canonical.com> Signed-off-by: Pauli Nieminen <ext-pauli.niemi...@nokia.com> --- hw/xfree86/dri2/dri2.c | 68 ++++++++++++++++++++++++++++++++++++------------ hw/xfree86/dri2/dri2.h | 1 + 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index c9fd74f..6a3f0cb 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -109,8 +109,10 @@ typedef struct _DRI2Screen { } DRI2ScreenRec; typedef struct _DRI2SwapCompleteDataRec { - ClientPtr client; - void *data; + DRI2BufferPtr src; + DRI2BufferPtr dest; + ClientPtr client; + void * data; } DRI2SwapCompleteDataRec, *DRI2SwapCompleteDataPtr; static DRI2ScreenPtr @@ -119,6 +121,23 @@ DRI2GetScreen(ScreenPtr pScreen) return dixLookupPrivate(&pScreen->devPrivates, dri2ScreenPrivateKey); } +static void +buffer_ref(DRI2BufferPtr buffer) +{ + buffer->refcnt++; +} + +static void +buffer_unref(DRI2DrawablePtr pPriv, DRI2BufferPtr buffer) +{ + DRI2ScreenPtr ds = pPriv->dri2_screen; + + buffer->refcnt--; + if (buffer->refcnt == 0) { + (*ds->DestroyBuffer)(pPriv, buffer); + } +} + DRI2DrawablePtr DRI2GetDrawable(DrawablePtr pDraw) { @@ -337,7 +356,6 @@ DRI2DestroyDrawable(ClientPtr client, DRI2DrawablePtr pPriv, XID id) static int DRI2DrawableGone(pointer p, XID id) { DRI2DrawablePtr pPriv = p; - DRI2ScreenPtr ds = pPriv->dri2_screen; DRI2DrawableRefPtr ref, next; DrawablePtr pDraw = pPriv->drawable; int i; @@ -383,7 +401,7 @@ static int DRI2DrawableGone(pointer p, XID id) if (pPriv->buffers != NULL) { for (i = 0; i < pPriv->bufferCount; i++) - (*ds->DestroyBuffer)(pPriv, pPriv->buffers[i]); + buffer_unref(pPriv, pPriv->buffers[i]); free(pPriv->buffers); } @@ -424,6 +442,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; @@ -438,14 +457,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)(pPriv, pPriv->buffers[i]); - } + if (pPriv->buffers[i] != NULL) + buffer_unref(pPriv, pPriv->buffers[i]); } free(pPriv->buffers); @@ -581,7 +598,7 @@ err_out: for (i = 0; i < count; i++) { if (buffers[i] != NULL) - (*ds->DestroyBuffer)(pPriv, buffers[i]); + buffer_unref(pPriv, buffers[i]); } free(buffers); @@ -793,6 +810,14 @@ DRI2WakeClient(ClientPtr client, DRI2DrawablePtr pPriv, int frame, } } +static void +free_swap_complete_data (DRI2DrawablePtr pPriv, DRI2SwapCompleteDataPtr pSwapData) +{ + buffer_unref(pPriv, pSwapData->src); + buffer_unref(pPriv, pSwapData->dest); + free(pSwapData); +} + void DRI2SwapComplete2(DRI2DrawablePtr pPriv, int frame, unsigned int tv_sec, unsigned int tv_usec, int type, @@ -807,11 +832,6 @@ DRI2SwapComplete2(DRI2DrawablePtr pPriv, int frame, pPriv->swap_count++; pPriv->refcnt--; - if (pPriv->refcnt <= 0) { - DRI2DrawableGone(pPriv, 0); - return; - } - if (pDraw) { BoxRec box; RegionRec region; @@ -835,7 +855,10 @@ DRI2SwapComplete2(DRI2DrawablePtr pPriv, int frame, DRI2WakeClient(client, pPriv, frame, tv_sec, tv_usec); - free(pSwapData); + free_swap_complete_data(pPriv, pSwapData); + + if (pPriv->refcnt <= 0) + DRI2DrawableGone(pPriv, 0); } Bool @@ -890,6 +913,17 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc, 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->client = client; pSwapData->data = data; @@ -949,11 +983,11 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc, pPriv->swapsPending++; pPriv->refcnt++; 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 */ pPriv->refcnt--; - free(pSwapData); + free_swap_complete_data(pPriv, pSwapData); xf86DrvMsg(pScreen->myNum, X_ERROR, "[DRI2] %s: driver failed to schedule swap\n", __func__); return BadDrawable; diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index 88171be..7b49e7f 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -44,6 +44,7 @@ typedef struct { unsigned int flags; unsigned int format; void *driverPrivate; + unsigned int refcnt; } DRI2BufferRec, *DRI2BufferPtr; struct _DRI2Drawable; -- 1.7.0.4 _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
