Bug 26394

Update:

I first made sure that my application only used GLX >=1.3 API. This would still crash. I then made sure it only used GLX <1.3, and now I have no crashes anymore. I'm planning to write a simple program that reproduces the crash/bug.

Created attachment 34041 [details]
Reproduces the crash

Here is a program that should crash the server eventually (just run it a few times, it might not happen every time). Also, it seems to happen more often if the program is exited the normal way (e.g. the window close button) than if it is killed (with some signal).

I think it is interesting that the server seems to *not* crash if the make context current call is removed.

Another thing, I now use version 2:1.7.5-1 in Debian.

Now please, someone reply. Currently, the only thing I've got for sending in this report is spam to my email address.

Is your program using indirect rendering?

Created attachment 34244 [details]
glxinfo output

glxinfo says "direct rendering: Yes".

Adding this piece of code just after the context is created in the example program:

printf ("Direct: %s\n", (glXIsDirect (dpy, glxcontext) == True) ? "yes" : "no");

Gives output "yes" in both the cases when it does not crash and the cases it
does crash.

I have attached the output of glxinfo too.

Exiting any Clutter 1.2 application causes the same crash. I have the same environment as the reporter but newer:

xserver-xorg-core: 2:1.7.5.902-1
xserver-xorg-video-intel: 2:2.10.903-1
linux-image-2.6.33-2-amd64: 2.6.33-1~experimental.4

I have tested running the program (attached before, with modification to print "Direct: yes/no") on my other computer (same server version, but software rasterizer). The program crashes, but not the server. The program outputs the following:

failed to create drawable
Direct: yes
Segmentation fault

Happens also when I run remote programs (it also says "Direct: yes" in that case, which makes no sense, I think).

Top of backtrace:

#0  0xb77d305f in XCreateDrawable (psc=0x80aa2f8, xDrawable=56623137, 
    drawable=56623137, modes=0x80b4480) at drisw_glx.c:89
        gcvalues = {function = 3, plane_mask = 3450467868, 
          foreground = 3540936910, background = 1915168984, 
          line_width = -928079311, line_style = -2102111902, 
          cap_style = 1901201503, join_style = -1946909669, 
          fill_style = 504982920, fill_rule = -1981880903, 
          arc_mode = 742396610, tile = 3411661115, stipple = 3456143762, 
          ts_x_origin = 2135257643, ts_y_origin = -1073749304, 
          font = 3221217988, subwindow_mode = -1073749324, 
          graphics_exposures = 0, clip_x_origin = 63, clip_y_origin = 1, 
          clip_mask = 3221218224, dash_offset = 1870652674, 
          dashes = -48 '\320'}
        visTemp = {visual = 0xbfffe494, visualid = 0, screen = 0, 
          depth = 134938528, class = 56623137, red_mask = 3221218296, 
          green_mask = 3078446832, blue_mask = 56623137, 
          colormap_size = 134934184, bits_per_rgb = -1073748968}
        num_visuals = 0
#1  driCreateDrawable (psc=0x80aa2f8, xDrawable=56623137, drawable=56623137, 
    modes=0x80b4480) at drisw_glx.c:321
        pdraw = <value optimized out>
        swrast = <value optimized out>
#2  0xb77b175e in FetchDRIDrawable (dpy=<value optimized out>, 
    glxDrawable=56623137, gc=0x80aeea8) at glxcurrent.c:294
        priv = <value optimized out>
        pdraw = <value optimized out>
        psc = <value optimized out>
#3  0xb77b1ab6 in MakeContextCurrent (dpy=0x8060a30, draw=56623137, 
    read=56623137, gc=0x80aeea8) at glxcurrent.c:370
        pdraw = 0xb7fff697
        pread = <value optimized out>
        reply = {type = 14 '\016', unused = 0 '\000', sequenceNumber = 0, 
          length = 0, contextTag = 134521636, pad2 = 1, pad3 = 3078236324, 
          pad4 = 134934184, pad5 = 3221218996, pad6 = 134899480}
        oldGC = 0xb77f77e0
        opcode = 151 '\227'
        oldOpcode = <value optimized out>
        bindReturnValue = <value optimized out>
        state = <value optimized out>
#4  0x08048dd9 in configure ()
...

In my previous comment I meant to say "run the program remotely" instead of "run remote programs".

*** Bug 27326 has been marked as a duplicate of this bug. ***

from bug 27326 - this issue is also affecting Clutter 1.2, since we switched to GLX 1.3 API.

Here is a clearer backtrace of the crash in case it helps.

Program received signal SIGSEGV, Segmentation fault.
0x0807fdae in privateExists (privates=0x14f, key=0xffa0fc) at ../../dix/privates.c:79
79	    return *key && *privates &&
(gdb) bt full
#0  0x0807fdae in privateExists (privates=0x14f, key=0xffa0fc) at ../../dix/privates.c:79
No locals.
#1  0x0807ffd1 in dixLookupPrivate (privates=0x14f, key=0xffa0fc) at ../../dix/privates.c:162
        ptr = 0x4f0
#2  0x00ff6bf9 in DRI2GetScreen (pScreen=0xffffffff) at ../../../../hw/xfree86/dri2/dri2.c:78
No locals.
#3  0x00ff731b in DRI2DestroyDrawable (pDraw=0xbae9110) at ../../../../hw/xfree86/dri2/dri2.c:344
        ds = 0xa25c568
        pPriv = 0x0
        pWin = 0xffa100
        pPixmap = 0xbae9490
#4  0x0056c7f9 in __glXDRIdrawableDestroy (drawable=0xbae93c0) at ../../glx/glxdri2.c:110
        private = 0xbae93c0
        core = 0x129bd40
#5  0x0055cd31 in DrawableGone (glxPriv=0xbae93c0, xid=67108869) at ../../glx/glxext.c:163
        c = 0x0
#6  0x08094ab3 in FreeClientResources (client=0xb9ec3a8) at ../../dix/resource.c:806
        rtype = 54
        head = 0xb9ec47c
        resources = 0xb9ec468
        this = 0xbae9a10
        j = 5
#7  0x0808fa43 in CloseDownClient (client=0xb9ec3a8) at ../../dix/dispatch.c:3631
        really_close_down = 1
#8  0x08087bc8 in Dispatch () at ../../dix/dispatch.c:424
        clientReady = 0xb8feac8
        result = -1
        client = 0xb9ec3a8
        nready = 0
        icheck = 0x82594fc
        start_tick = 720
#9  0x08066d33 in main (argc=8, argv=0xbfd8efd4, envp=0xbfd8eff8) at ../../dix/main.c:285
        i = 1
        alwaysCheckForInput = {0, 1}

#2  0x00ff6bf9 in DRI2GetScreen (pScreen=0xffffffff)

That doesn't look good.

The backtraces in comments #6 and #10 are not congruent, are these actually the same crash?  #10 matches the original reported trace so perhaps is the one to focus on.

(In reply to comment #11)
> 
> The backtraces in comments #6 and #10 are not congruent, are these actually the
> same crash?  #10 matches the original reported trace so perhaps is the one to
> focus on.
> 

The back trace in comment #6 is not from the server, but the previously attached example program that reproduces the bug. When I run it on my laptop with Intel chip the server crashes, but when I run it on my desktop with software rasterizer the server survives but the program crashes.

Applying this commit to debian/ubuntu's xserver 1.7.6 fixes the server segfault when closing clutter apps (with clutter 1.2.4) for me, but the problem still remains with the testcase on this bug. Both cases have identical backtraces to the one in comment 10, and forcing swrast with LIBGL_ALWAYS_SOFTWARE=1 gives an identical backtrace and experience as comment #6 for the test case on comment #2. I should mention I am using intel as well as is everyone I have seen reporting this so far.

http://cgit.freedesktop.org/xorg/xserver/commit/?id=fa5103a02bd509e4a102afdad2ab26cb22210367

dri2: No need to blit from front on DRI2GetBuffers if they're just being reused.
It can be quite an expensive operation, so we're better off not doing
it unless it's totally required.

Signed-off-by: Francisco Jerez <currojerez@riseup.net>
Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>

The fix suggested in the previous comment does not resolve the crashes in clutter 1.2.4 here. Sorry :/

Can you try this mesa fix?

diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
index c4b5cb9..27a700d 100644
--- a/src/glx/dri2_glx.c
+++ b/src/glx/dri2_glx.c
@@ -164,7 +164,7 @@ dri2DestroyDrawable(__GLXDRIdrawable * pdraw)
    const __DRIcoreExtension *core = pdraw->psc->core;
 
    (*core->destroyDrawable) (pdraw->driDrawable);
-   DRI2DestroyDrawable(pdraw->psc->dpy, pdraw->xDrawable);
+   DRI2DestroyDrawable(pdraw->psc->dpy, pdraw->drawable);
    Xfree(pdraw);
 }

Created attachment 34628 [details] [review]
Make DRI2DestroyDrawable more robust if pDraw is invalid

If pDraw is invalid (i.e. we already freed the priv) don't try to use it to look up the screen.

I think this is just a workaround... any comments Kristian?

*** Bug 27406 has been marked as a duplicate of this bug. ***

(In reply to comment #16)
> Created an attachment (id=34628) [details]
> Make DRI2DestroyDrawable more robust if pDraw is invalid
> 
> If pDraw is invalid (i.e. we already freed the priv) don't try to use it to
> look up the screen.
> 
> I think this is just a workaround... any comments Kristian?

With this patch in a ubuntu lucid environment (xserver 1.7.6, intel 2.9.1+backports, mesa 7.7.1) it still happens, but the probability of occurance is greatly reduced instead of being guaranteed at every closure of quadrapassel. With xorg-edgers packages (mesa 7.9, intel 2.11) on lucid as well as this xserver patch it completely goes away though.

I don't see(In reply to comment #16)
> Created an attachment (id=34628) [details]
> Make DRI2DestroyDrawable more robust if pDraw is invalid
> 
> If pDraw is invalid (i.e. we already freed the priv) don't try to use it to
> look up the screen.

I don't see how that can work - if the WindowRec pointed to by pDraw was already freed, DRI2GetDrawable() can't be relied upon.

See e.g. commits 7b6400a1b8d2f228fcbedf17c30a7e3924e4dd2a ('glx: Fix drawable private leak on destroy'), 2075d4bf9e53b8baef0b919da6c44771220cd4a5 ('glx: If a destroyed window is bound to the current context, make it not current.') and 3020b1d43e34fca08cd51f7c7c8ed51497d49ef3 ('glx: Clean up more thoroughly if the drawable of a current context goes away.') for how similar issues were dealt with previously.

BTW, it may not be immediately related to this problem, but I suspect commit 711e26466ae04ae93ff4c48d377d83d68a6320e9 ('DRI2: handle drawable destruction properly at DRI2SwapComplete time') can't work properly for windows - it tries to defer the drawable destruction, but that's not possible for windows.

Yeah, I know it's just a bad workaround.  It makes the crashes much less frequent but definitely doesn't address the real problem.  Thanks for the heads up on the window destruction too; I think more of this code needs to use resource IDs and lookups for tracking rather than the drawable pointers.  Eric made a change like this to the intel driver...

I wonder if there's a better way of tracking the GLX drawable vs X window drawable handles in both the Mesa and X code; it seems like we've been confusing them a lot lately.

Created attachment 34687 [details] [review]
Make sure corresponding X drawable exists before trying to use it

Looked over this some more with Kristian today.  It looks like the root of the problem is that the GLX drawable we're destroying in __glXdrawableDestroy has a stale corresponding X drawable, which was destroyed in the GLX DestroyWindow hook (added by 120286aef59dabdb7c9fa762e08457e5cc8ec3a6).

That'll make the X drawable go away, but we'll be left with a pDraw pointing into freed space, probably crashing when we get to __glXdrawableDestroy for the actual GLX drawable in question.

This patch does a lookup on the X drawable ID corresponding to the GLX drawable at destruction time, and ignores it if it no longer exists.  Kristian put together an alternate approach as well but it needs a little work: http://people.freedesktop.org/~krh/glx-mess.patch.

And per the last comment, DRI2 priv destruction also needs some fixing; I think it's now safe to always free the priv and clear the private wherever we call DRI2FreeDrawable, since we check everywhere we need to whether the priv still exists and so shouldn't crash.

(In reply to comment #21)
> Created an attachment (id=34687) [details]
> Make sure corresponding X drawable exists before trying to use it
> 
> Looked over this some more with Kristian today.  It looks like the root of the
> problem is that the GLX drawable we're destroying in __glXdrawableDestroy has a
> stale corresponding X drawable, which was destroyed in the GLX DestroyWindow
> hook (added by 120286aef59dabdb7c9fa762e08457e5cc8ec3a6).
> 
> That'll make the X drawable go away, but we'll be left with a pDraw pointing
> into freed space, probably crashing when we get to __glXdrawableDestroy for the
> actual GLX drawable in question.
> 
> This patch does a lookup on the X drawable ID corresponding to the GLX drawable
> at destruction time, and ignores it if it no longer exists.  Kristian put
> together an alternate approach as well but it needs a little work:
> http://people.freedesktop.org/~krh/glx-mess.patch.
> 
> And per the last comment, DRI2 priv destruction also needs some fixing; I think
> it's now safe to always free the priv and clear the private wherever we call
> DRI2FreeDrawable, since we check everywhere we need to whether the priv still
> exists and so shouldn't crash.

Jesse, I tested your patch and it fixed the X crash issue in MeeGo. Thank you.

*** Bug 27473 has been marked as a duplicate of this bug. ***

(In reply to comment #21)
> Looked over this some more with Kristian today.  It looks like the root of the
> problem is that the GLX drawable we're destroying in __glXdrawableDestroy has a
> stale corresponding X drawable, which was destroyed in the GLX DestroyWindow
> hook (added by 120286aef59dabdb7c9fa762e08457e5cc8ec3a6).

glxDestroyWindow() itself doesn't destroy the DrawableRec, but as I've pointed out over and over again, the destruction of a window indeed cannot be deferred.

> That'll make the X drawable go away, but we'll be left with a pDraw pointing
> into freed space, probably crashing when we get to __glXdrawableDestroy for the
> actual GLX drawable in question.

The purpose of glxDestroyWindow() was to clean up such dangling references. Apparently it's missing some of them (now).

> This patch does a lookup on the X drawable ID corresponding to the GLX drawable
> at destruction time, and ignores it if it no longer exists.  Kristian put
> together an alternate approach as well but it needs a little work:
> http://people.freedesktop.org/~krh/glx-mess.patch.

Yeah, when we discussed this kind of issue earlier, Kristian was against using IDs everywhere, and I think there are good arguments for not doing that.

> And per the last comment, DRI2 priv destruction also needs some fixing; I think
> it's now safe to always free the priv and clear the private wherever we call
> DRI2FreeDrawable, since we check everywhere we need to whether the priv still
> exists and so shouldn't crash.

If you're referring to the last paragraph of comment #19, I'm not sure that'll do. If there are outstanding swaps referencing the DrawableRec of a window being destroyed, those swaps will need to be cancelled or whatever is necessary to clean up the dangling references.

(In reply to comment #24)
> (In reply to comment #21)
> > Looked over this some more with Kristian today.  It looks like the root of the
> > problem is that the GLX drawable we're destroying in __glXdrawableDestroy has a
> > stale corresponding X drawable, which was destroyed in the GLX DestroyWindow
> > hook (added by 120286aef59dabdb7c9fa762e08457e5cc8ec3a6).
> 
> glxDestroyWindow() itself doesn't destroy the DrawableRec, but as I've pointed
> out over and over again, the destruction of a window indeed cannot be deferred.

Right, thus this patch.

> > That'll make the X drawable go away, but we'll be left with a pDraw pointing
> > into freed space, probably crashing when we get to __glXdrawableDestroy for the
> > actual GLX drawable in question.
> 
> The purpose of glxDestroyWindow() was to clean up such dangling references.
> Apparently it's missing some of them (now).
> 
> > This patch does a lookup on the X drawable ID corresponding to the GLX drawable
> > at destruction time, and ignores it if it no longer exists.  Kristian put
> > together an alternate approach as well but it needs a little work:
> > http://people.freedesktop.org/~krh/glx-mess.patch.
> 
> Yeah, when we discussed this kind of issue earlier, Kristian was against using
> IDs everywhere, and I think there are good arguments for not doing that.

Such as?  Do you have ideas for an alternate approach?  Or do you like Kristian's proposed patch better?

> 
> > And per the last comment, DRI2 priv destruction also needs some fixing; I think
> > it's now safe to always free the priv and clear the private wherever we call
> > DRI2FreeDrawable, since we check everywhere we need to whether the priv still
> > exists and so shouldn't crash.
> 
> If you're referring to the last paragraph of comment #19, I'm not sure that'll
> do. If there are outstanding swaps referencing the DrawableRec of a window
> being destroyed, those swaps will need to be cancelled or whatever is necessary
> to clean up the dangling references.

I don't know how to handle this yet, I'll look at it some more...

(In reply to comment #25)
> Such as?  Do you have ideas for an alternate approach?  Or do you like
> Kristian's proposed patch better?

I'll leave the argument to Kristian, as he was previously arguing against patches of mine using ID lookup, but I thought the alternative approach should be clear by now: Clean up all references to the DrawableRec of a window being destroyed.

(In reply to comment #26)
> (In reply to comment #25)
> > Such as?  Do you have ideas for an alternate approach?  Or do you like
> > Kristian's proposed patch better?
> 
> I'll leave the argument to Kristian, as he was previously arguing against
> patches of mine using ID lookup, but I thought the alternative approach should
> be clear by now: Clean up all references to the DrawableRec of a window being
> destroyed.

Ok I'll chat with Kristian.  The main problem with the cleanup approach is that it's a bit more invasive than the small patch here, and seems to violate some of the existing GLX layering; I was hoping you had something clever to make the impact small.  I agree that we'll add some lookup overhead, I'm just not sure how much impact that will have.

Not to derail, but I just wanted to say that the patch in comment #21 did fix all 4 machines I've tested in a ubuntu lucid environment. Thank you!

There's a xserver package here for anyone else experiencing this on lucid and wanting to test it. The publisher is being a little slow today so it may not show up for a bit.

https://edge.launchpad.net/~sarvatt/+archive/bugfixes

(In reply to comment #24)
> (In reply to comment #21)
> > Looked over this some more with Kristian today.  It looks like the root of the
> > problem is that the GLX drawable we're destroying in __glXdrawableDestroy has a
> > stale corresponding X drawable, which was destroyed in the GLX DestroyWindow
> > hook (added by 120286aef59dabdb7c9fa762e08457e5cc8ec3a6).u
> 
> glxDestroyWindow() itself doesn't destroy the DrawableRec, but as I've pointed
> out over and over again, the destruction of a window indeed cannot be deferred.
> 
> > That'll make the X drawable go away, but we'll be left with a pDraw pointing
> > into freed space, probably crashing when we get to __glXdrawableDestroy for the
> > actual GLX drawable in question.
> 
> The purpose of glxDestroyWindow() was to clean up such dangling references.
> Apparently it's missing some of them (now).

It (120286aef59dabdb7c9fa762e08457e5cc8ec3a6) always missed GLX windows created using glXCreateWindow.  Those have a different XID than the X window.  For the DestroyWindow hook to work correctly, we have to store the __GLXdrawable as a private on the window, look it up in the hook and then call FreeResource on the glx drawable XID.

> > This patch does a lookup on the X drawable ID corresponding to the GLX drawable
> > at destruction time, and ignores it if it no longer exists.  Kristian put
> > together an alternate approach as well but it needs a little work:
> > http://people.freedesktop.org/~krh/glx-mess.patch.
> 
> Yeah, when we discussed this kind of issue earlier, Kristian was against using
> IDs everywhere, and I think there are good arguments for not doing that.

The idea was mostly to avoid the lookup overhead - not that it's a too expensive operation, but since we already looked up the drawable pointer once we might as well hold on to it.  The other concern is robustness in case of reused XIDs.

> > And per the last comment, DRI2 priv destruction also needs some fixing; I think
> > it's now safe to always free the priv and clear the private wherever we call
> > DRI2FreeDrawable, since we check everywhere we need to whether the priv still
> > exists and so shouldn't crash.
> 
> If you're referring to the last paragraph of comment #19, I'm not sure that'll
> do. If there are outstanding swaps referencing the DrawableRec of a window
> being destroyed, those swaps will need to be cancelled or whatever is necessary
> to clean up the dangling references.

Before 711e26466ae04ae93ff4c48d377d83d68a6320e9, the callback mechanism was designed so that the we would hold on to the DRI2 private and pass it back in the DRI2SwapComplete callback.  While we can't prevent a window from being destroyed, we can make the DRI2 private ref-counted and keep it alive long enough that any outstanding flips finish.  If a window goes away while we have a flip pending, we can NULL the DrawablePtr in the DRI2 private. When the DRI2SwapComplete eventually happens, the DRI2 private is still valid, and will allow us to reliably discover that the window is gone.  That mechanism worked well enough I think, but the problem was that we didn't always check to see if the drawable was gone in the callback.  Jesse is saying that that's now fixed an we can roll back 711e26466ae04ae93ff4c48d377d83d68a6320e9.

Created attachment 34730 [details] [review]
Alternate fix

To sum up, what I'd recommend is that we pick up Francisco's patch:

  http://lists.freedesktop.org/archives/xorg-devel/2010-February/005525.html

and add the patch attached here on top to properly destroy GLX drawables in the DestroyWindow hook, whether they were created through the compatibility mechanism or glXCreateWindow().  Compared to Jesses patch, this doesn't have issues with reused XID (not sure that it's a big issue though).

If we go with Jesses patch we should be able to just drop the DestroyWindow hook, ie revert 120286aef59dabdb7c9fa762e08457e5cc8ec3a6.

Reassigning to Kristian as he is pushing the fix.

Created attachment 35001 [details] [review]
DRI2: Track DRI2 drawables as resources, not privates

The main motivation here is to have the resource system clean up the
DRI2 drawable automatically so glx doesn't have to.  Right now, the
glx drawable resource must be destroyed before the X drawable, so that
calling DRI2DestroyDrawable doesn't crash.  By making the DRI2
drawable a resource, GLX doesn't have to worry about that and the
resource destruction order becomes irrelevant.

https://bugs.freedesktop.org/show_bug.cgi?id=26394

Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>

Created attachment 35002 [details] [review]
glx: Drop DestroyWindow hook

Now that glx doesn't call DRI2DestroyDrawable anymore, we don't need to
force a specific resource destruction order in the DestroyWindow hook.

Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>

Created attachment 35003 [details] [review]
DRI2: Track DRI2 drawables as resources, not privates

The main motivation here is to have the resource system clean up the
DRI2 drawable automatically so glx doesn't have to.  Right now, the
glx drawable resource must be destroyed before the X drawable, so that
calling DRI2DestroyDrawable doesn't crash.  By making the DRI2
drawable a resource, GLX doesn't have to worry about that and the
resource destruction order becomes irrelevant.

https://bugs.freedesktop.org/show_bug.cgi?id=26394

Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>

Created attachment 35004 [details] [review]
glx: Drop DestroyWindow hook

Now that glx doesn't call DRI2DestroyDrawable anymore, we don't need to
force a specific resource destruction order in the DestroyWindow hook.

Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>

Created attachment 35005 [details] [review]
DRI2: Track DRI2 drawables as resources, not privates

The main motivation here is to have the resource system clean up the
DRI2 drawable automatically so glx doesn't have to.  Right now, the
glx drawable resource must be destroyed before the X drawable, so that
calling DRI2DestroyDrawable doesn't crash.  By making the DRI2
drawable a resource, GLX doesn't have to worry about that and the
resource destruction order becomes irrelevant.

https://bugs.freedesktop.org/show_bug.cgi?id=26394

Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>

Created attachment 35006 [details] [review]
glx: Drop DestroyWindow hook

Now that glx doesn't call DRI2DestroyDrawable anymore, we don't need to
force a specific resource destruction order in the DestroyWindow hook.

Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>

Attachment 35005 [details] and attachment 35006 [details] [review] are the latest proposed fix.  They've been posted to xorg-devel for review, blocking on keithp to review and commit at this point.

The patches are now on xserver master, please give that a try and close the bug if it fixes the problem for you.

Kristian:

I have a question about the patch "DRI2: Track DRI2 drawables as resources, not privates", it calls AddResource() in DRI2CreateDrawable() to add DRI2 drawable as resources, but where we free them, should we call FreeResourceByType() to free the DRI2 drawable in ProcDRI2DestroyDrawable() ?

Also in my test in MeeGo. I find that in ProcDRI2CreateDrawable(), sometimes the value of "pDrawable->id" is 0, which is different with stuff->drawable. that would cause some problem. because we will get an old DRI2 drawable from DRI2GetDrawable() that attached to the zero value of "pDrawable->id". I think we need to add some check to make sure "pDrawable->id" is synced with "stuff->drawable".

I attached a draft patch that addressing above two points, please review.

Created attachment 35191 [details] [review]
DRI2-drawable-id-check-and -free-resource.patch

with the below 4 commits in xserver, I don't see crash anymore.

glx: Drop DestroyWindow hook
DRI2: Track DRI2 drawables as resources, not privates
glx: Let the resource system destroy pixmaps
glx: Track GLX 1.3 style GLX drawables under their X drawable ID as well

Created attachment 35195 [details] [review]
Also handle AIGLX

This patch I posted to the xorg-devel list also handles the DRI2CreateDrawable() call in __glXDRIscreenCreateDrawable(). However, I'm not sure it's valid to just assign the pixmap id field like this.

See bug 27767 for one possible symptom of this problem.

*** Bug 28380 has been marked as a duplicate of this bug. ***

Closing based on comments.