2010/4/16 Michel Dänzer <[email protected]>: > On Thu, 2010-04-15 at 12:13 -0400, Kristian Høgsberg wrote: >> On Thu, 15 Apr 2010 15:48:00 +0200, Michel Dänzer <[email protected]> wrote: >> > On Thu, 2010-04-15 at 08:26 -0400, Kristian Høgsberg wrote: >> > > 2010/4/15 Kristian Høgsberg <[email protected]>: >> > > > 2010/4/15 Michel Dänzer <[email protected]>: >> > > >> On Thu, 2010-04-15 at 07:58 -0400, Kristian Høgsberg wrote: >> > > >>> 2010/4/15 Michel Dänzer <[email protected]>: >> > > >>> > On Wed, 2010-04-14 at 15:13 -0400, Kristian Høgsberg wrote: >> > > >>> >> >> > > >>> >> The real fix is the patch from attachment 25038, not the >> > > >>> >> DestroyWindow >> > > >>> >> hook. If the context is destroyed first, it will remove itself >> > > >>> >> from >> > > >>> >> the glxAllContexts list so the DrawableGone destructor won't >> > > >>> >> touch it. >> > > >>> >> On the other hand, if the drawable is destroyed first, thanks to >> > > >>> >> your >> > > >>> >> patch, it will detach itself from the context properly so context >> > > >>> >> destruction (whether at resource cleanup time or at client >> > > >>> >> shutdown >> > > >>> >> time) wont touch a free drawable. >> > > >>> > >> > > >>> > Right, but that will only work if DestroyWindow of the X window >> > > >>> > also >> > > >>> > destroys the corresponding GLX drawable. Are you saying that's >> > > >>> > guaranteed anyway now, and have you tested that none of MacSlow's >> > > >>> > rgba-glx demos crashes the X server on exit anymore without this >> > > >>> > hook? >> > > >>> >> > > >>> Yes, the GLX drawable is a client allocated resource that gets >> > > >>> cleaned >> > > >>> up when the client exits. >> > > >> >> > > >> And if it doesn't? :) What's to stop a client from binding a window >> > > >> to a >> > > >> GLX context, destroying the X window and then doing more stuff with >> > > >> the >> > > >> GLX context? >> > > > >> > > > We handle that in DrawableGone. You wrote that patch. >> > > >> > > Just to clarify, when a window is destroyed, all resources with the >> > > same ID are destroyed. So as long as there are no dependencies >> > > between resource destructors for the different types of resources with >> > > that ID, there should be no problem. >> > >> > It all sounds great in theory, but I still don't understand why the hook >> > was necessary (or at least made a difference) when I added it, but that >> > should no longer be the case now. :( Is it that the GLX drawable could >> > have a different ID back then but no longer can now? >> >> I just ran master X server (that is, with the DestroyWindow hook in and >> neither of the two patches in this series) and it crashes when I exit >> rgba-glx by pressing escape. Reading the patch and the resource code >> again, I can't see that the hook ever did anything. The way resources >> are tracked, all resources with the same ID are in the same bucket in a >> linked list. Resources are prepended to the list and a window/pixmap is >> always the first resource added for a given ID. Destruction traverses >> the list from head to tail, so the RT_WINDOW resource is the last to be >> destroyed, and that's when the DestroyWindow hook is called. At that >> point there are no long any resources with the ID left in the bucket and >> FreeResources(), as called from glxDestroyWindow(), doesn't do anything. > > It's starting to make sense. Thanks for bearing with me, my
Likewise, I'm glad we got to the bottom of this. > Reviewed-by: Michel Dänzer <[email protected]> > > for this patch is well deserved. :) Thanks. >> The case you mentioned with using an GLX drawable after destroying the X >> drawable is indeed a problem. However, it only happens for GLX >> drawables created using the glXCreateWindow/Pixmap functions. The >> problem is that the DrawableGone hook doesn't get called for the GLX >> drawable when the window is destroyed, since the GLX drawable has a >> different ID. Neither my DRI2-drawables-as-resources or the hook fixes >> this. I confirmed the problem by adding a glXSwapBuffer() call to >> rgba-glx after the XDestroyWindow() call and running it in indirect mode >> (in direct rendering the DRI2 protocol code throws an BadDrawable error >> before we get to dereference any freed drawables). The server crashes. >> The patch below fixes the problem. > > Reviewed-by: Michel Dänzer <[email protected]> > > for this one as well. Cool, resent the patch series with the patch from the thread with Keith about how to clean up the pbuffer pixmap properly. Kristian _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
