[PATCH v2 1/2] DRI2: Free DRI2 drawable references in DRI2DestroyDrawable

[Pauli Nieminen]

If client calls DRI2CreateDrawable multiple times for same drawable
DRI2 creates multiple references. Multiple references cause DRI2 send
multiple invalidate events for same client.

Problem is easy to see in xtrace. Like following filtered snippet from
problematic client:
000:<:0b85: 16: DRI2-Request(132,5): GetBuffers drawable=0x00800011
attachments={attachment=BackLeft(0x00000001)};
000:<:0b89: 32: DRI2-Request(132,8): SwapBuffers drawable=0x00800011
target_msc_hi=0 target_msc_lo=0 divisor_hi=0 divisor_lo=0 remainder_hi=0
remainder_lo=0
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:<:0b8e: 16: DRI2-Request(132,5): GetBuffers drawable=0x00800011
attachments={attachment=BackLeft(0x00000001)};
000:<:0b8f: 32: DRI2-Request(132,8): SwapBuffers drawable=0x00800011
target_msc_hi=0 target_msc_lo=0 divisor_hi=0 divisor_lo=0 remainder_hi=0
remainder_lo=0
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
000:<:0bc4: 16: DRI2-Request(132,5): GetBuffers drawable=0x00800011
attachments={attachment=BackLeft(0x00000001)};
000:<:0bc5: 32: DRI2-Request(132,8): SwapBuffers drawable=0x00800011
target_msc_hi=0 target_msc_lo=0 divisor_hi=0 divisor_lo=0 remainder_hi=0
remainder_lo=0

Problem is triggered because client side EGL implementation is using
DRI2 directly.

DRI2 code in server doesn't handle DRI2DestroyDrawable that leaks
references. If client recreates rendering target EGL destroys and
creates DRI2 drawable. In that case DRI2DestroyDrawable leaks DRI2
drawable references.

To fix this memory leak/performance problem server has to free the
reference when client requests for it.

V2:
* Simplified DRI2DestroyDrawable based on Kristian's review
* Split refcnt to different patch

Signed-off-by: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
CC: Rami Ylimäki <rami.ylimaki at vincit.fi>
CC: Kristian Høgsberg <krh at bitplanet.net>

---
 hw/xfree86/dri2/dri2.c    |   31 +++++++++++++++++++++++++++++++
 hw/xfree86/dri2/dri2.h    |    4 +++-
 hw/xfree86/dri2/dri2ext.c |    2 +-
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 34f735f..75af3ea 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -236,6 +236,18 @@ DRI2AddDrawableRef(DRI2DrawablePtr pPriv, XID id, XID dri2_id,
     return Success;
 }
 
+static DRI2DrawableRefPtr
+DRI2FindClientDrawableRef(ClientPtr client, DRI2DrawablePtr pPriv)
+{
+	DRI2DrawableRefPtr ref;
+
+	list_for_each_entry(ref, &pPriv->reference_list, link) {
+		if (CLIENT_ID(ref->dri2_id) == client->index)
+			return ref;
+	}
+	return NULL;
+}
+
 int
 DRI2CreateDrawable(ClientPtr client, DrawablePtr pDraw, XID id,
 		   DRI2InvalidateProcPtr invalidate, void *priv)
@@ -258,6 +270,25 @@ DRI2CreateDrawable(ClientPtr client, DrawablePtr pDraw, XID id,
     return Success;
 }
 
+int
+DRI2DestroyDrawable(ClientPtr client, DrawablePtr pDraw, XID id)
+{
+    DRI2DrawablePtr pPriv = DRI2GetDrawable(pDraw);
+    DRI2DrawableRefPtr ref;
+
+    if (!pPriv)
+	return BadDrawable;
+
+    ref = DRI2FindClientDrawableRef(client, pPriv);
+
+    if (!ref)
+	return BadDrawable;
+
+    FreeResourceByType(ref->dri2_id, dri2DrawableRes, FALSE);
+
+    return Success;
+}
+
 static int DRI2DrawableGone(pointer p, XID id)
 {
     DRI2DrawablePtr pPriv = p;
diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
index fe0bf6c..dd63c30 100644
--- a/hw/xfree86/dri2/dri2.h
+++ b/hw/xfree86/dri2/dri2.h
@@ -214,7 +214,9 @@ extern _X_EXPORT int DRI2CreateDrawable(ClientPtr client,
 					DRI2InvalidateProcPtr invalidate,
 					void *priv);
 
-extern _X_EXPORT void DRI2DestroyDrawable(DrawablePtr pDraw);
+extern _X_EXPORT int DRI2DestroyDrawable(ClientPtr client,
+                                         DrawablePtr pDraw,
+                                         XID id);
 
 extern _X_EXPORT DRI2BufferPtr *DRI2GetBuffers(DrawablePtr pDraw,
 			     int *width,
diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
index 4e48e65..c7e6be1 100644
--- a/hw/xfree86/dri2/dri2ext.c
+++ b/hw/xfree86/dri2/dri2ext.c
@@ -200,7 +200,7 @@ ProcDRI2DestroyDrawable(ClientPtr client)
 		       &pDrawable, &status))
 	return status;
 
-    return Success;
+    return DRI2DestroyDrawable(client, pDrawable, stuff->drawable);
 }
 
 
-- 
1.7.0.4