Philip Martin wrote on Tue, Aug 21, 2012 at 18:00:23 +0100:
> The FS loader uses a vtable fs_library_vtable_t defined in fs-loader.h.
> Some of the functions defined in the vtable have a common_pool
> parameter:
>
> /* The open_fs/create/open_fs_for_recovery/upgrade_fs functions are
> serialized so that they may use the common_pool parameter to
> allocate fs-global objects such as the bdb env cache. */
> svn_error_t *(*create)(svn_fs_t *fs, const char *path, apr_pool_t *pool,
> apr_pool_t *common_pool);
> svn_error_t *(*open_fs)(svn_fs_t *fs, const char *path, apr_pool_t *pool,
> apr_pool_t *common_pool);
>
>
> In FSFS this common_pool gets passed to fs_serialized_init in fs.c where
> it is used to setup setup mutexes:
>
> status = apr_pool_userdata_get(&val, key, common_pool);
> if (status)
> return svn_error_wrap_apr(status, _("Can't fetch FSFS shared data"));
> ffsd = val;
>
> if (!ffsd)
> {
> ffsd = apr_pcalloc(common_pool, sizeof(*ffsd));
> ffsd->common_pool = common_pool;
>
> /* POSIX fcntl locks are per-process, so we need a mutex for
> intra-process synchronization when grabbing the repository write
> lock. */
> SVN_ERR(svn_mutex__init(&ffsd->fs_write_lock,
> SVN_FS_FS__USE_LOCK_MUTEX, common_pool));
>
>
> Functions like fs_library_vtable_t.pack_fs and
> fs_library_vtable_t.recover that don't take the common_pool parameter
> pass the pool parameter to fs_serialized_init. I think that means these
> functions should only be used from a single threaded process.
>
> Is that correct? If so we should document it or change the functions to
> handle the common_pool.
>
I think the correct fix is to propagate common_pool to have fs_pack()
and pass it as to the appropriate parameter of fs_serialized_init().
> I'm not sure how the absence of common_pool affects the BDB
> implementations.
Is it even used? A quick grep in libsvn_fs_base suggests that only fs.c
has variables by that name; therein, only svn_fs_base__init() uses them;
and that function seems to have no callers(!?).
(These results are so odd that I'm pretty sure I typoed the argument to
grep at some point...)
