On 05.04.2012 00:44, Philip Martin wrote:
> Philip Martin <philip.martin_at_wandisco.com> writes:
>
>> I think there is a refcount/locking bug in svn_fs_bdb__close. This code
>>
>> - if (0 == --bdb_baton->error_info->refcount && bdb->pool)
>> - {
>> - svn_error_clear(bdb_baton->error_info->pending_errors);
>> -#if APR_HAS_THREADS
>> - free(bdb_baton->error_info);
>> - apr_threadkey_private_set(NULL, bdb->error_info);
>> -#endif
>> - }
>>
>> should be inside svn_fs_bdb__close_internal protected by the
>> bdb_cache_lock otherwise the error_info refcount can change while
>> another thread is inside svn_fs_bdb__open_internal and holding the lock.
> That's not correct--the error data is per-thread and so there should be
> no race. Except ...
>
>> However moving the code from __close to __close_internal so it is inside
>> the lock doesn't stop the tests failing so there must be a second bug
>> somewhere.
> ... I think I may have identified the problem. The patch below checks
> that the error struct is allocated and released by the same thread.
> With the worker MPM the assertion always passes but with the event MPM I
> get assertion failures. I don't fully understand the effect this would
> have but I suspect it explains the problem.
I think this pretty much explains what you're seeing. It was years ago,
but I do recall that, when I implemented DB_REGISTER handling, I made
the internal BDB handles thread-specific, which means that you aren't
allowed to use the same handle from different threads at any time --
that's a side effect of using thread-specific storage for certain
"global" data.
If event MPM hands off requests between different threads, than either
mod_dav_svn has to make sure that only the actual worker thread opens
the FS handle, or the BDB code itself needs to change to account for the
difference between "specific to one thread" and "one thread at a time."
Sorry but I've no idea which approach would be easier, apart from having
a feeling of dread sweep over me at the idea of touching that BDB handle
management code. :)
-- Brane
Received on 2012-04-05 05:17:12 CEST