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

Re: [TSVN] Question about 2560

From: SteveKing <stefankueng_at_gmail.com>
Date: 2005-01-28 13:06:26 CET

On Fri, 28 Jan 2005 11:00:34 +0000, Will Dean <svn@indcomp.co.uk> wrote:

> The crash is caused because in TortoiseProc.cpp, we do an svn.ReleasePool()
> after the first delete failure. This clears 'pool', which has some
> important things in it like (particularly) the config, loaded with
> svn_config_get_config, and referenced by m_ctx.config.
>
> If we clear this pool, then m_ctx.config points at free'd memory and when,
> down in the svn_client_delete call, SVN tries to look at its config, it
> crashes.

The bug is not that SVN::ReleasePool() frees the pool! The problem is
that we allocated the config stuff in pool and not parentpool (which
doesn't get freed by ReleasePool()).
But you're right, my 'fix' was not right.

> I believe that SVN::ReleasePool is fundamentally broken, in that it leaves
> the SVN object irrepairably damaged, with m_ctx referencing various free'd
> objects.
>
> But it *is* necessary to release the pool which svn_client_delete has used
> between each call, because otherwise the ADM cleanups don't get run, and
> the second delete will fail saying that the WC is locked.
>
> I have made the following changes:
>
> Backed-out 2560
> Removed SVN:ReleasePool altogether
> Made svn_client_delete use a subpool, so that whatever happens, it
> cleans-up after itself.

Good.

> I don't believe that there is a requirement for something like the pathlist
> array to be allocated from the *same* pool as the client function
> uses. (If I'm wrong on this, there are lots of broken functions in our SVN
> class!)
>
> However, I *am* currently concerned that the error structure which is
> returned by svn_client functions is allocated on the pool that the function
> call is given. If this is the case, then we *do* have a lot of broken
> functions which are setting the svn_error_t pointer to point to subpool
> memory which is free'd very shortly afterwards.

No. The error baton is always allocated in the topmost pool. That is in our
case parentpool.

> What do y'all think...?

I've changed the following:
- allocate the config stuff in parentpool instead of pool
- use the recommended svn_client_create_context() API to initialize the context
  instead of our own initialisation (which was required when
svn_client_create_context()
  wasn''t available yet).

Stefan

-- 
       ___
  oo  // \\      "De Chelonian Mobile"
 (_,\/ \_/ \     TortoiseSVN
   \ \_/_\_/>    The coolest Interface to (Sub)Version Control
   /_/   \_\     http://tortoisesvn.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tortoisesvn.tigris.org
For additional commands, e-mail: dev-help@tortoisesvn.tigris.org
Received on Fri Jan 28 13:08:09 2005

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.