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

Re: Another 1.4 release critical bug

From: Garrett Rooney <rooneg_at_electricjellyfish.net>
Date: 2006-08-03 15:26:09 CEST

On 8/2/06, Branko ╚ibej <brane@xbc.nu> wrote:

> Heh, you just proposed what I proposed. :)
> Basically, for the BDB back-end, we're talking about a significant part
> of the code. Not to mention that doing this would violate the layering.

Last night another solution occurred to me.

So, if we need the DSOs to stick around as long as possible, longer
than any pool that could potentially hold an svn_fs_t, then why not
just put them in the global pool? Each pool has a pointer to its
parent, so we just need to climb up that chain until we hit the root.
It's not like we're sticking a huge amount of data in there, it's
bounded by the total number of FS backends, so we should be safe...

The actual change is pretty trivial:

Index: subversion/libsvn_fs/fs-loader.c
===================================================================
--- subversion/libsvn_fs/fs-loader.c (revision 20940)
+++ subversion/libsvn_fs/fs-loader.c (working copy)
@@ -263,6 +263,28 @@

 /* --- Functions for operating on filesystems by pathname --- */

+/* Walk the list of pools upward from POOL until we hit the global pool,
+ and return it. */
+static apr_pool_t *
+find_global_pool(apr_pool_t *pool)
+{
+ for (;;)
+ {
+ apr_pool_t *parent = apr_pool_parent_get(pool);
+
+ /* So, if we're at the top either we'll get a NULL parent back,
+ or we'll get the same pool. Most operating systems return NULL,
+ but on Netware you get back the same pool because the global
+ pool is REALLY global (i.e. common to all apps running on the
+ system, so APR won't return it to you. */
+
+ if (! parent || parent == pool)
+ return pool;
+
+ pool = parent;
+ }
+}
+
 svn_error_t *
 svn_fs_initialize(apr_pool_t *pool)
 {
@@ -274,7 +296,11 @@
   if (common_pool)
     return SVN_NO_ERROR;

- common_pool = svn_pool_create(NULL);
+ /* So, we absolutely positively need the DSOs we allocate in this pool
+ to live as long as possible. As a result we need to use the global
+ pool, the last one that's destroyed at apr_terminate time. */
+ common_pool = find_global_pool(pool);
+
   dso_cache = apr_hash_make(common_pool);
 #if APR_HAS_THREADS
   status = apr_thread_mutex_create(&common_pool_lock,

Let me know if I'm missing something here, otherwise I'll keep playing
around with this idea and hopefully commit something later.

-garrett
Received on Thu Aug 3 15:29:59 2006

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.