You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@collab.net> on 2006/07/18 17:05:49 UTC
Re: svn commit: r20717 - trunk/subversion/libsvn_fs_base/bdb
On Tue, 18 Jul 2006, rooneg@tigris.org wrote:
...
> Fix yet another global pool destruction ordering bug in the bdb cache.
>
> * subversion/libsvn_fs_base/bdb/env.c
> (svn_fs_bdb__close): Don't bother removing entries from the bdb cache
> if bdb_cache_lock is NULL, since that indicates that we're already in
> global pool destruction time and the bdb cache pool has already been
> destroyed.
...
> --- trunk/subversion/libsvn_fs_base/bdb/env.c (original)
> +++ trunk/subversion/libsvn_fs_base/bdb/env.c Tue Jul 18 09:43:53 2006
> @@ -603,7 +603,11 @@
> }
> else
> {
> - apr_hash_set(bdb_cache, &bdb->key, sizeof bdb->key, NULL);
> + /* If the bdb cache lock has been set to NULL that means we are
> + shutting down, and the pool that holds the bdb cache has already
> + been destroyed, so accessing it here would be a Bad Thing (tm) */
> + if (bdb_cache_lock)
> + apr_hash_set(bdb_cache, &bdb->key, sizeof bdb->key, NULL);
> err = bdb_close(bdb);
> release_cache_mutex();
> }
Is there a race condition between when we check whether bdb_cache_lock
is non-null and when we use it?
Re: svn commit: r20717 - trunk/subversion/libsvn_fs_base/bdb
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 7/18/06, Daniel Rall <dl...@collab.net> wrote:
> On Tue, 18 Jul 2006, rooneg@tigris.org wrote:
> ...
> > Fix yet another global pool destruction ordering bug in the bdb cache.
> >
> > * subversion/libsvn_fs_base/bdb/env.c
> > (svn_fs_bdb__close): Don't bother removing entries from the bdb cache
> > if bdb_cache_lock is NULL, since that indicates that we're already in
> > global pool destruction time and the bdb cache pool has already been
> > destroyed.
> ...
> > --- trunk/subversion/libsvn_fs_base/bdb/env.c (original)
> > +++ trunk/subversion/libsvn_fs_base/bdb/env.c Tue Jul 18 09:43:53 2006
> > @@ -603,7 +603,11 @@
> > }
> > else
> > {
> > - apr_hash_set(bdb_cache, &bdb->key, sizeof bdb->key, NULL);
> > + /* If the bdb cache lock has been set to NULL that means we are
> > + shutting down, and the pool that holds the bdb cache has already
> > + been destroyed, so accessing it here would be a Bad Thing (tm) */
> > + if (bdb_cache_lock)
> > + apr_hash_set(bdb_cache, &bdb->key, sizeof bdb->key, NULL);
> > err = bdb_close(bdb);
> > release_cache_mutex();
> > }
>
> Is there a race condition between when we check whether bdb_cache_lock
> is non-null and when we use it?
No, there shouldn't be any race, since it's only ever set to NULL at
global pool destruction time, which we assume happens at atexit time,
where everything is single threaded.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org