Re: svn commit:r1555350-/subversion/trunk/subversion/libsvn_client/mtcc.c

On Sun, Jan 5, 2014 at 9:58 AM, Bert Huijben <bert_at_qqmail.nl> wrote:

> Note that I don't disagree with the mtcc change/fix.
>
> But I think we must fix the reason why this pattern causes problems in
> fsfs, fsx, etc.
>

Well, as already pointed out, apr pools default to strict LIFO.
The good thing about this is that it "just works" if you don't
try to do anything asymmetric. Whenever you do something
else, you need to be extra careful.

So, based on Brane's comments, I did some digging and found
that the Ev1 implementation of the commit editor did not close
the txn root object in close_edit() / abort_edit(). r1555499 fixed it.

Before that, I looked at how svn_fs_close_root would work on
txn roots and discovered a pool usage issue introduced in 1.7 .
Fixed in r1555491.

The combination of both patches makes the original mucc
code work - so all sorted. FWIW, this was a ra_local issue only.

We only test extensively Subversion with short lived client tools and in
> some ways svnmucc is an exception there... But so are TortoiseSVN,
> AnkhSvn, Subclipse, etc. Etc.
>
> That all our command line tools had to be patched shows a pattern to a
> bigger problem of new pool lifetime issues that we should really fix before
> 1.9 or we will have many segfaults to fix for the 1.9 patch releases.
>

When did we have to "patch all cl tools"? The fact that the
code fixed above has been around for several releases without
massive problems shows that being strictly LIFO is a good
enough pattern.

OTOH, we always had the occasional pool lifetime issue
and fixed it. I don't see a massive problem there.

I'm mildly worried by the fact that apr pools are not thread-safe
by default and that making them thread-safe by default in SVN
would massively hurt our performance due to the number of
small pools we are using. Hence all the fiddling with pools
in svnserve.

-- Stefan^2.
Received on 2014-01-05 21:48:53 CET