Re: Subversion BDB doesn't work with Apache 2.4 event MPM

Philip Martin <philip.martin_at_wandisco.com> writes:

> ==10434== Thread 15:
> ==10434== Invalid read of size 4
> ==10434== at 0x802D5BB: svn_fs_bdb__open_internal (env.c:660)
> ==10434== by 0x802D679: svn_fs_bdb__open (env.c:672)
> ==10434== by 0x80390D7: open_databases (fs.c:536)
> ==10434== by 0x8039C26: base_open (fs.c:763)
> ==10434== by 0x77445A5: svn_fs_open (fs-loader.c:374)
> ==10434== by 0x752D8C6: get_repos (repos.c:1416)
> ==10434== by 0x752DA13: svn_repos_open2 (repos.c:1462)
> ==10434== by 0x72EBB1B: get_resource (repos.c:2159)
> ==10434== by 0x70B7B73: dav_get_resource (mod_dav.c:712)
> ==10434== by 0x70BC768: dav_method_options (mod_dav.c:1602)
> ==10434== by 0x70BDAE7: dav_handler (mod_dav.c:4706)
> ==10434== by 0x44BBBF: ap_run_handler (config.c:169)
> ==10434== Address 0x17a0b690 is 16 bytes inside a block of size 24 free'd
> ==10434== at 0x4C240FD: free (vg_replace_malloc.c:366)
> ==10434== by 0x802D0BF: svn_fs_bdb__close (env.c:539)
> ==10434== by 0x8038AAA: cleanup_fs (fs.c:183)
> ==10434== by 0x8038B36: cleanup_fs_apr (fs.c:289)
> ==10434== by 0x508DBCD: apr_pool_clear (apr_pools.c:2359)
> ==10434== by 0x669ADE3: process_lingering_close (event.c:1253)
> ==10434== by 0x669B987: listener_thread (event.c:1485)
> ==10434== by 0x58F18C9: start_thread (pthread_create.c:300)
> ==10434== by 0x600286C: clone (clone.S:112)

I'm not familiar with this code. I can "fix" the problem by removing
the free() from svn_fs_bdb__close but that's not really surprising given
the warnings above, nor is it a solution. The patch below also appears
to fix the problem but I'm reluctant to commit it until I understand why
this problem only occurs with the event MPM.

Index: subversion/libsvn_fs_base/bdb/env.c
===================================================================
--- subversion/libsvn_fs_base/bdb/env.c (revision 1308769)
+++ subversion/libsvn_fs_base/bdb/env.c (working copy)
@@ -517,6 +517,9 @@ svn_fs_bdb__close_internal(bdb_env_t *bdb)
   return svn_error_trace(err);
 }
 
+static apr_status_t
+cleanup_env_baton(void *data);
+
 svn_error_t *
 svn_fs_bdb__close(bdb_env_baton_t *bdb_baton)
 {
@@ -524,6 +527,9 @@ svn_fs_bdb__close(bdb_env_baton_t *bdb_baton)
 
   SVN_ERR_ASSERT(bdb_baton->env == bdb_baton->bdb->env);
 
+ apr_pool_cleanup_kill(bdb_baton->pool_for_cleanup, bdb_baton,
+ cleanup_env_baton);
+
   /* Neutralize bdb_baton's pool cleanup to prevent double-close. See
      cleanup_env_baton(). */
   bdb_baton->bdb = NULL;
@@ -660,6 +666,7 @@ svn_fs_bdb__open_internal(bdb_env_baton_t **bdb_ba
   ++(*bdb_batonp)->error_info->refcount;
   apr_pool_cleanup_register(pool, *bdb_batonp, cleanup_env_baton,
                             apr_pool_cleanup_null);
+ (*bdb_batonp)->pool_for_cleanup = pool;
 
   return SVN_NO_ERROR;
 }
Index: subversion/libsvn_fs_base/bdb/env.h
===================================================================
--- subversion/libsvn_fs_base/bdb/env.h (revision 1308769)
+++ subversion/libsvn_fs_base/bdb/env.h (working copy)
@@ -86,6 +86,9 @@ typedef struct bdb_env_baton_t
 
   /* The error info related to this baton. */
   bdb_error_info_t *error_info;
+
+ apr_pool_t *pool_for_cleanup;
+
 } bdb_env_baton_t;
 

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com