On Mon, 2011-03-28 at 11:03 -0700, Keith Packard wrote: > On Mon, 28 Mar 2011 10:56:08 -0400, Adam Jackson <[email protected]> wrote: > > > No code to create objects of this type ever existed. > > I suspect that's a bug then -- the damage object is going to hang around > with a dead pointer to the window when it is destroyed. > > Here's a completely untested patch that hooks the window id to the > damage record so that it is destroyed when the window is destroyed: > > I think this gets the ENOMEM case right -- AddResource will call > FreeDamageExtWin if it fails, which will call FreeResource on the damage > ID, freeing the damage structure.
This may be more correct in that it cleans up the Damage. I think it's
less correct in that you break existing clients. FreeDamageExtWin()
deletes the Damage XID, but the spec mentions nothing about magically
garbage-collecting Damages just because their drawable went away. From
a quick check, it looks like mutter pushes an error handler around
XDamageDestroy(), but compiz and kwin do not.
Given that, I think we have to make it so damage actions against a
Damage that has lost its Drawable simply fizzle, and continue to leave
cleanup to the clients. Compiled, not tested:
diff --git a/damageext/damageext.c b/damageext/damageext.c
index 754383d..72611ec 100644
--- a/damageext/damageext.c
+++ b/damageext/damageext.c
@@ -30,7 +30,7 @@
static unsigned char DamageReqCode;
static int DamageEventBase;
static RESTYPE DamageExtType;
-static RESTYPE DamageExtWinType;
+static RESTYPE DamageExtDrawableType;
static DevPrivateKeyRec DamageClientPrivateKeyRec;
#define DamageClientPrivateKey (&DamageClientPrivateKeyRec)
@@ -89,6 +89,9 @@ DamageExtReport (DamagePtr pDamage, RegionPtr pRegion, void
*closure)
{
DamageExtPtr pDamageExt = closure;
+ if (pDamageExt->drawable == None)
+ return;
+
switch (pDamageExt->level) {
case DamageReportRawRegion:
case DamageReportDeltaRegion:
@@ -220,6 +223,9 @@ ProcDamageCreate (ClientPtr client)
DamageSetReportAfterOp (pDamageExt->pDamage, TRUE);
DamageRegister (pDamageExt->pDrawable, pDamageExt->pDamage);
+ if (!AddResource(pDrawable->id, DamageExtDrawableType, pDamageExt))
+ return BadAlloc;
+
if (pDrawable->type == DRAWABLE_WINDOW)
{
pRegion = &((WindowPtr) pDrawable)->borderClip;
@@ -446,7 +452,7 @@ FreeDamageExt (pointer value, XID did)
*/
pDamageExt->id = 0;
if (WindowDrawable(pDamageExt->pDrawable->type))
- FreeResourceByType (pDamageExt->pDrawable->id, DamageExtWinType, TRUE);
+ FreeResourceByType (pDamageExt->pDrawable->id, DamageExtDrawableType,
TRUE);
if (pDamageExt->pDamage)
{
DamageUnregister (pDamageExt->pDrawable, pDamageExt->pDamage);
@@ -457,12 +463,12 @@ FreeDamageExt (pointer value, XID did)
}
static int
-FreeDamageExtWin (pointer value, XID wid)
+FreeDamageExtDrawable (pointer value, XID wid)
{
DamageExtPtr pDamageExt = (DamageExtPtr) value;
- if (pDamageExt->id)
- FreeResource (pDamageExt->id, RT_NONE);
+ pDamageExt->drawable = None;
+
return Success;
}
@@ -497,8 +503,9 @@ DamageExtensionInit(void)
if (!DamageExtType)
return;
- DamageExtWinType = CreateNewResourceType (FreeDamageExtWin,
"DamageExtWin");
- if (!DamageExtWinType)
+ DamageExtDrawableType = CreateNewResourceType (FreeDamageExtDrawable,
+ "DamageExtDrawable");
+ if (!DamageExtDrawableType)
return;
if (!dixRegisterPrivateKey(&DamageClientPrivateKeyRec, PRIVATE_CLIENT,
sizeof (DamageClientRec)))
---
- ajax
signature.asc
Description: This is a digitally signed message part
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
