[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r1211162 - /subversion/trunk/subversion/libsvn_wc/context.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Wed, 7 Dec 2011 14:23:09 +0100

> -----Original Message-----
> From: Bert Huijben [mailto:bert_at_qqmail.nl]
> Sent: woensdag 7 december 2011 11:42
> To: dev_at_subversion.apache.org; hwright_at_apache.org
> Subject: RE: svn commit: r1211162 -
> /subversion/trunk/subversion/libsvn_wc/context.c
>
>
>
> > -----Original Message-----
> > From: hwright_at_apache.org [mailto:hwright_at_apache.org]
> > Sent: dinsdag 6 december 2011 22:22
> > To: commits_at_subversion.apache.org
> > Subject: svn commit: r1211162 -
> > /subversion/trunk/subversion/libsvn_wc/context.c
> >
> > Author: hwright
> > Date: Tue Dec 6 21:22:08 2011
> > New Revision: 1211162
> >
> > URL: http://svn.apache.org/viewvc?rev=1211162&view=rev
> > Log:
> > Fix a potential memory cleanup ordering bug.
>
> This introduces too many other cleanup problems. This makes the context
> and db unavailable from the normal cleanup handlers in the same pool, that
> will run as normal cleanup.
>
> The wc_db is carefull to not make assumptions and allocates everything in
> the same pool and *this* change breaks that by closing the db earlier than
> those cleanups.
>
> -0.5 (open for discussion).
>
>
> If not extremely necessary we should never use a pre-cleanup handler in our
> code as it just *starts* a race on who can cleanup first and thereby pushes
> the segfaults in other code
>
> There should be no race: everything should be implemented as a proper last-
> in, first out.

One (of many) examples is how the the update editor uses pools.
This editor installs (or installed) cleanup handlers to make sure it doesn't forget to run the workingqueue.

This change (or similar changes in other places) will close the wc_db before the subpools had the option to access the database for their cleanup.

Please revert this change and fix the real cause of the 'memory cleanup ordering bug' you describe. This change introduces too many other reordering bugs, which will be impossible to fix without introducing things like pre-pre-post-pre cleanup handlers.

The only thing that works understandable for pool cleanup is a proper last-in-first-out. If somebody bends the rules by going out of order, other locations will have to do the same thing. And in the end everybody has to go this way :(

>
> Bert
Received on 2011-12-07 14:23:45 CET

This is an archived mail posted to the Subversion Dev mailing list.