Philip Martin <philip.mar...@wandisco.com> writes:

> 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;

That makes the tests run a bit further but they still fail, valgrind
shows:

==22500== Thread 15:
==22500== Invalid read of size 4
==22500==    at 0x802D639: svn_fs_bdb__open_internal (env.c:663)
==22500==    by 0x802D706: svn_fs_bdb__open (env.c:676)
==22500==    by 0x8039167: open_databases (fs.c:536)
==22500==    by 0x8039CB6: base_open (fs.c:763)
==22500==    by 0x77445A5: svn_fs_open (fs-loader.c:374)
==22500==    by 0x752D8C6: get_repos (repos.c:1416)
==22500==    by 0x752DA13: svn_repos_open2 (repos.c:1462)
==22500==    by 0x72EBB1B: get_resource (repos.c:2159)
==22500==    by 0x70B7B73: dav_get_resource (mod_dav.c:712)
==22500==    by 0x70B8FB1: dav_method_propfind (mod_dav.c:2010)
==22500==    by 0x70BDAF7: dav_handler (mod_dav.c:4710)
==22500==    by 0x44BBBF: ap_run_handler (config.c:169)
==22500==  Address 0x184d9500 is 16 bytes inside a block of size 24 free'd
==22500==    at 0x4C240FD: free (vg_replace_malloc.c:366)
==22500==    by 0x802D179: svn_fs_bdb__close (env.c:555)
==22500==    by 0x8038B3A: cleanup_fs (fs.c:183)
==22500==    by 0x8038BC6: cleanup_fs_apr (fs.c:289)
==22500==    by 0x508DBCD: apr_pool_clear (apr_pools.c:2359)
==22500==    by 0x669ADE3: process_lingering_close (event.c:1253)
==22500==    by 0x669B987: listener_thread (event.c:1485)
==22500==    by 0x58F18C9: start_thread (pthread_create.c:300)
==22500==    by 0x600286C: clone (clone.S:112)
==22500== 

Which looks like a similar problem with another pool cleanup at a higher
level.

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

Reply via email to