xserver: Branch 'xorg-server-1.4-apple' - 2 commits
George Peter Staplin
gstaplin at kemper.freedesktop.org
Wed Mar 4 01:07:06 PST 2009
hw/xquartz/xpr/dri.c | 61 +++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 50 insertions(+), 11 deletions(-)
New commits:
commit aa0a57996f3e7d16238f69976958c2526821388b
Author: George Staplin <gstaplin at apple.com>
Date: Wed Mar 4 02:03:52 2009 -0700
XQuartz: Add diagnostic error checking to xp_destroy_surface.
This occurred to me in hindsight after the last commit. If the
original developer had done this, we would have noticed the
problem sooner.
diff --git a/hw/xquartz/xpr/dri.c b/hw/xquartz/xpr/dri.c
index e4e290b..99857b3 100644
--- a/hw/xquartz/xpr/dri.c
+++ b/hw/xquartz/xpr/dri.c
@@ -858,8 +858,13 @@ DRISurfaceNotify(xp_surface_id id, int kind)
if (kind == AppleDRISurfaceNotifyDestroyed)
{
- xp_destroy_surface(pDRIDrawablePriv->sid);
+ xp_error error;
+ error = xp_destroy_surface(pDRIDrawablePriv->sid);
+
+ if(error)
+ ErrorF("%s: xp_destroy_surface failed: %d\n", __func__, error);
+
/* Guard against reuse, even though we are freeing after this. */
pDRIDrawablePriv->sid = 0;
commit bede83eb19a1629396fcd5a46441f8476a8fcd1b
Author: George Staplin <gstaplin at apple.com>
Date: Wed Mar 4 01:39:58 2009 -0700
XQuartz: Fix a memory leak with surfaces that a new test found.
xp_destroy_surface was called with a surface id of 0, due to some
premature cleanup that set it to 0. This means the surfaces
weren't being destroyed until the window was.
The code that did that was: pDRIDrawablePriv->sid = 0;
In long running applications this leak may or may not have been
harmful. With the old libGL the surfaces weren't destroyed until
the context was destroyed or a new context created. In the new
libGL they are reference counted, and released much sooner, so we
ran into a resource leak more noticeably with some tests.
Make the Apple DRI code dispatch events to the client(s) for
destroyed surfaces, when a resource is destroyed. This seems to
work in my tests, however this clearly wasn't working for a while,
so bugs may result in the future if it enables some new (unexpected)
side effects.
Also add a few helpful comments to aid in understanding the code
in the future.
Tested with the test suite, Pymol, and various Mesa demos.
diff --git a/hw/xquartz/xpr/dri.c b/hw/xquartz/xpr/dri.c
index 3a6f87e..e4e290b 100644
--- a/hw/xquartz/xpr/dri.c
+++ b/hw/xquartz/xpr/dri.c
@@ -520,8 +520,8 @@ DRICreateSurface(ScreenPtr pScreen, Drawable id,
return FALSE; /*error*/
}
#endif
- else { /* for GLX 1.3, a PBuffer */
- /* NOT_DONE */
+ else {
+ /* We handle GLXPbuffers in a different way (via CGL). */
return FALSE;
}
@@ -611,13 +611,27 @@ DRIDestroySurface(ScreenPtr pScreen, Drawable id, DrawablePtr pDrawable,
}
if (pDRIDrawablePriv != NULL) {
+ /*
+ * This doesn't seem to be used, because notify is NULL in all callers.
+ */
+
if (notify != NULL) {
pDRIDrawablePriv->notifiers = x_hook_remove(pDRIDrawablePriv->notifiers,
notify, notify_data);
}
- if (--pDRIDrawablePriv->refCount <= 0) {
- /* This calls back to DRIDrawablePrivDelete
- which frees the private area */
+
+ --pDRIDrawablePriv->refCount;
+
+ /*
+ * Check if the drawable privates still have a reference to the
+ * surface.
+ */
+
+ if (pDRIDrawablePriv->refCount <= 0) {
+ /*
+ * This calls back to DRIDrawablePrivDelete which
+ * frees the private area and dispatches events, if needed.
+ */
FreeResourceByType(id, DRIDrawablePrivResType, FALSE);
}
}
@@ -625,6 +639,10 @@ DRIDestroySurface(ScreenPtr pScreen, Drawable id, DrawablePtr pDrawable,
return TRUE;
}
+/*
+ * The assumption is that this is called when the refCount of a surface
+ * drops to <= 0, or the window/pixmap is destroyed.
+ */
Bool
DRIDrawablePrivDelete(pointer pResource, XID id)
{
@@ -643,18 +661,23 @@ DRIDrawablePrivDelete(pointer pResource, XID id)
}
if (pDRIDrawablePriv == NULL) {
+ /*
+ * We reuse __func__ and the resource type for the GLXPixmap code.
+ * Attempt to free a pixmap buffer associated with the resource
+ * if possible.
+ */
return DRIFreePixmapImp(pDrawable);
}
-
+
if (pDRIDrawablePriv->drawableIndex != -1) {
/* release drawable table entry */
pDRIPriv->DRIDrawables[pDRIDrawablePriv->drawableIndex] = NULL;
}
if (pDRIDrawablePriv->sid != 0) {
- xp_destroy_surface(pDRIDrawablePriv->sid);
- x_hash_table_remove(surface_hash, x_cvt_uint_to_vptr(pDRIDrawablePriv->sid));
+ DRISurfaceNotify(pDRIDrawablePriv->sid, AppleDRISurfaceNotifyDestroyed);
}
+
if (pDRIDrawablePriv->notifiers != NULL)
x_hook_free(pDRIDrawablePriv->notifiers);
@@ -803,6 +826,11 @@ DRIQueryVersion(int *majorVersion,
*patchVersion = APPLE_DRI_PATCH_VERSION;
}
+/*
+ * Note: this also cleans up the hash table in addition to notifying clients.
+ * The sid/surface-id should not be used after this, because it will be
+ * invalid.
+ */
void
DRISurfaceNotify(xp_surface_id id, int kind)
{
@@ -823,21 +851,27 @@ DRISurfaceNotify(xp_surface_id id, int kind)
if (kind == AppleDRISurfaceNotifyDestroyed)
{
- pDRIDrawablePriv->sid = 0;
- x_hash_table_remove(surface_hash, x_cvt_uint_to_vptr(id));
+ x_hash_table_remove(surface_hash, x_cvt_uint_to_vptr(id));
}
x_hook_run(pDRIDrawablePriv->notifiers, &arg);
if (kind == AppleDRISurfaceNotifyDestroyed)
{
- /* Kill off the handle. */
+ xp_destroy_surface(pDRIDrawablePriv->sid);
+
+ /* Guard against reuse, even though we are freeing after this. */
+ pDRIDrawablePriv->sid = 0;
FreeResourceByType(pDRIDrawablePriv->pDraw->id,
DRIDrawablePrivResType, FALSE);
}
}
+/*
+ * This creates a shared memory buffer for use with GLXPixmaps
+ * and AppleSGLX.
+ */
Bool DRICreatePixmap(ScreenPtr pScreen, Drawable id,
DrawablePtr pDrawable, char *path,
size_t pathmax)
More information about the xorg-commit
mailing list