This patch looks good to me Leif. Alan.
On Fri, Feb 14, 2003 at 06:24:50 -0500, Leif Delgass wrote: > On Fri, 14 Feb 2003, David Dawes wrote: > > [snip] > > > There are some more serious things holding up 4.3, including the issue > > that Leif mentioned here a couple of days ago. I haven't seen anyone > > comment on his proposed solution for that one either... > > > > David > > I was wondering about that myself. :) I'll reiterate the problem, but this > time with a patch to give people something more concrete to comment on. > This is, I think, the easiest and quickest fix, if not necessarily the > most correct. This patch does the following: > > 1. Move the __driGarbageCollectDrawables() call in driDestroyContext() > back after the driver's DestroyContext() callback as in the original patch > in the DRI mesa-4-0-4-branch. This avoids the issue of the drawable being > destroyed before the driver tries to reference it, at least in the > observed case in the Radeon driver's DestroyContext(). > > 2. Free pBackClipRects in driDestroyDrawable(). This fixes a potential > memory leak if there are back-buffer cliprects when the drawable is > destroyed. I'm pretty sure this a seperate issue and a valid fix. > > 3. As an added sanity measure, set pClipRects and pBackClipRects pointers > to NULL in the drawable private struct when they are freed, before freeing > the struct. This _might_ prevent a mirrored private drawable struct > pointer held by a driver from pointing to invalid cliprect pointers. > However, it won't change the fact that the mirrored private drawable > struct pointer itself would be invalid. This change is pretty dubious, as > it has the potential to hide the actual problem. I don't think this > should take the place of fix #1. If the mirrored private drawable pointer > itself is invalid, dereferencing it produces undefined behavior. > > If we don't use Fix #1, the alternative is to modify the drivers to set > the mirrored drawable pointer to NULL in the DestroyBuffer() callback and > deal with the possibility of a NULL drawable pointer in any code that can > be called after DestroyBuffer(), e.g when locking and any state changes > made after the lock is acquired. This might be a more correct fix, but it > seems to me it would be more intrusive and require more checking for > side-effects. > > -- > Leif Delgass > http://www.retinalburn.net Content-Description: dri_util.diff > Index: dri_util.c > =================================================================== > RCS file: /cvs/xc/lib/GL/dri/dri_util.c,v > retrieving revision 1.5 > diff -u -r1.5 dri_util.c > --- dri_util.c 2003/02/11 03:30:20 1.5 > +++ dri_util.c 2003/02/14 22:50:11 > @@ -629,7 +629,7 @@ > pdp->numClipRects = 0; > pdp->pClipRects = NULL; > pdp->numBackClipRects = 0; > - pdp->pBackClipRects = 0; > + pdp->pBackClipRects = NULL; > } > else > pdp->pStamp = &(psp->pSAREA->drawableTable[pdp->index].stamp); > @@ -740,8 +740,14 @@ > (*psp->DriverAPI.DestroyBuffer)(pdp); > if (__driWindowExists(dpy, pdp->draw)) > (void)XF86DRIDestroyDrawable(dpy, scrn, pdp->draw); > - if (pdp->pClipRects) > + if (pdp->pClipRects) { > Xfree(pdp->pClipRects); > + pdp->pClipRects = NULL; > + } > + if (pdp->pBackClipRects) { > + Xfree(pdp->pBackClipRects); > + pdp->pBackClipRects = NULL; > + } > Xfree(pdp); > } > } > @@ -763,8 +769,8 @@ > psp->fullscreen = NULL; > } > } > - __driGarbageCollectDrawables(pcp->driScreenPriv->drawHash); > (*pcp->driScreenPriv->DriverAPI.DestroyContext)(pcp); > + __driGarbageCollectDrawables(pcp->driScreenPriv->drawHash); > (void)XF86DRIDestroyContext(dpy, scrn, pcp->contextID); > Xfree(pcp); > } ------------------------------------------------------- This SF.NET email is sponsored by: FREE SSL Guide from Thawte are you planning your Web Server Security? Click here to get a FREE Thawte SSL guide and find the answers to all your SSL security issues. http://ads.sourceforge.net/cgi-bin/redirect.pl?thaw0026en _______________________________________________ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
