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

Re: svn commit: r17621 - trunk/subversion/bindings/swig/python/svn

From: David James <james_at_cs.toronto.edu>
Date: 2005-12-06 00:47:19 CET

On 12/5/05, Daniel Rall <dlr@collab.net> wrote:
> On Sun, 04 Dec 2005, djames@tigris.org wrote:
> ...
> > Log:
> > * subversion/bindings/swig/python/svn/core.py
> > (svn_pool_destroy, svn_pool_clear): Check whether pools are valid before
> > working with them. This change prevents assertion failures when Subversion
> > 1.2.x applications try to destroy pools during application shutdown, when
> > the validity of the pool is not guaranteed. Subversion 1.2.x applications
> > can't call "pool.valid()" themselves to check the validity of pools, because
> > it's a new API in Subversion 1.3.x.
> >
> > Notes: Fixes warnings about assertion failures in the test suite for
> > Trac 0.9.x
> ...
> > --- trunk/subversion/bindings/swig/python/svn/core.py (original)
> > +++ trunk/subversion/bindings/swig/python/svn/core.py Sun Dec 4 12:26:42 2005
> > @@ -185,7 +185,8 @@
> > assert pool is not None
> >
> > if hasattr(pool,"destroy"):
> > - pool.destroy()
> > + if pool.valid():
> > + pool.destroy()
> > else:
> > _core.apr_pool_destroy(pool)
> > apr_pool_destroy = svn_pool_destroy
> > @@ -198,7 +199,8 @@
> > assert pool is not None
> >
> > if hasattr(pool,"clear"):
> > - pool.clear()
> > + if pool.valid():
> > + pool.clear()
> > else:
> > _core.apr_pool_clear(pool)
> > apr_pool_clear = svn_pool_clear
>
> If the pool has a clear() or destroy() method, but is not valid(), it
> is okay to call neither the instance method nor _core's
> apr_pool_destroy()/apr_pool_clear() function? [...]

In r17621, I changed our compatibility pool functions for 1.2.x to not
call "destroy" on invalid pools, so that 1.2.x applications could
safely call svn_pool_destroy on pools during application shutdown, as
they could during Subversion 1.2. Upon second thought, I've realised
that my solution in r17621 is too drastic, because it deprives 1.2.x
applications of valuable error messages added in 1.3.x. (r17621 also
inadvertently broke our test suite, which expects our 1.2.x API to
report error messages.)

In r17638 and r17639, I've fixed the compatibility issue with
application shutdown using a different solution: I only suppress the
"svn_pool_destroy" error messages if the "application_pool" has
already been destroyed. Here's the new code for svn_pool_destroy().
Does this make more sense?

> assert pool is not None
>
> - pool.destroy()
> + # New in 1.3.x: All pools are automatically destroyed when Python shuts
> + # down. For compatibility with 1.2.x, we won't report an error if your
> + # app tries to destroy a pool during the shutdown process. Instead, we
> + # check to make sure the application_pool is still around before calling
> + # pool.destroy().
> + if application_pool:
> + pool.destroy()
>

Cheers,

David

--
David James -- http://www.cs.toronto.edu/~james
Received on Tue Dec 6 00:48:02 2005

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

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