On 01.05.2015 19:11, Philip Martin wrote:
> Philip Martin <philip.mar...@wandisco.com> writes:
>
>> Philip Martin <philip.mar...@wandisco.com> writes:
>>
>>> Index: ../src-1.9/subversion/libsvn_ra/ra_loader.c
>>> ===================================================================
>>> --- ../src-1.9/subversion/libsvn_ra/ra_loader.c (revision 1677108)
>>> +++ ../src-1.9/subversion/libsvn_ra/ra_loader.c (working copy)
>>> @@ -355,7 +355,7 @@ svn_error_t *svn_ra_open4(svn_ra_session_t **sessi
>>> /* Library not found. */
>>> continue;
>>>
>>> - SVN_ERR(initfunc(svn_ra_version(), &vtable, scratch_pool));
>>> + SVN_ERR(initfunc(svn_ra_version(), &vtable, sesspool));
>>>
>>> SVN_ERR(check_ra_version(vtable->get_version(), scheme));
>>>
>>> allows the initialize to work again.
>> For ra_local the initfunc is svn_ra_local__init and it contains a
>> comment:
>>
>> #ifndef SVN_LIBSVN_CLIENT_LINKS_RA_LOCAL
>> /* This assumes that POOL was the pool used to load the dso. */
>> SVN_ERR(svn_fs_initialize(pool));
>> #endif
>>
>> Passing scratch_pool breaks the assumption in the comment since
>> scratch_pool was not used to load the DSO.
> Perhaps something like this:
>
> Index: subversion/include/private/svn_subr_private.h
> ===================================================================
> --- subversion/include/private/svn_subr_private.h (revision 1677165)
> +++ subversion/include/private/svn_subr_private.h (working copy)
> @@ -681,6 +681,10 @@ svn_boolean_t
> svn_bit_array__get(svn_bit_array__t *array,
> apr_size_t idx);
>
> +/* Return the global pool used the DSO loader, this pool may be NULL. */
> +apr_pool_t *
> +svn_dso__pool(void);
> +
> /** @} */
>
> #ifdef __cplusplus
> Index: subversion/libsvn_ra_local/ra_plugin.c
> ===================================================================
> --- subversion/libsvn_ra_local/ra_plugin.c (revision 1677165)
> +++ subversion/libsvn_ra_local/ra_plugin.c (working copy)
> @@ -1869,7 +1869,7 @@ svn_ra_local__init(const svn_version_t *loader_ver
>
> #ifndef SVN_LIBSVN_CLIENT_LINKS_RA_LOCAL
> /* This assumes that POOL was the pool used to load the dso. */
> - SVN_ERR(svn_fs_initialize(pool));
> + SVN_ERR(svn_fs_initialize(svn_dso__pool()));
> #endif
>
> *vtable = &ra_local_vtable;
> Index: subversion/libsvn_subr/dso.c
> ===================================================================
> --- subversion/libsvn_subr/dso.c (revision 1677165)
> +++ subversion/libsvn_subr/dso.c (working copy)
> @@ -122,4 +122,11 @@ svn_dso_load(apr_dso_handle_t **dso, const char *f
>
> return SVN_NO_ERROR;
> }
> +
> +apr_pool_t *
> +svn_dso__pool(void)
> +{
> + return dso_pool;
> +}
> +
> #endif /* APR_HAS_DSO */
This looks like a much better solution, yes. Strictly speaking, we'll
"leak" the shared FSFS structs into the DSO pool, but would have to use
a really unreasonable number of repositories from a single process
before this becomes a problem.
+1
-- Brane
