On 02.07.2011 04:35, Blair Zajac wrote:
> On Jul 1, 2011, at 5:27 PM, stefan2_at_apache.org wrote:
>> Author: stefan2
>> Date: Sat Jul 2 00:27:28 2011
>> New Revision: 1142130
>> URL: http://svn.apache.org/viewvc?rev=1142130&view=rev
>> Replace usage of plain APR thread mutex API with the new svn_mutex
>> API in FSFS.
>> -#if APR_HAS_THREADS
>> - /* We also need a mutex for synchronising access to the active
>> - transaction list and free transaction pointer. */
>> - status = apr_thread_mutex_create(&ffsd->txn_list_lock,
>> - APR_THREAD_MUTEX_DEFAULT,
>> - if (status)
>> - return svn_error_wrap_apr(status,
>> - _("Can't create FSFS txn list
>> + SVN_ERR(svn_mutex__init(&ffsd->txn_list_lock, TRUE,
> I don't get why you have enable_mutex. On a platform without thread
> support, svn_mutex__init with TRUE will always return APR_ENOTIMPL.
> The proper fix is to wrap APR_HAS_THREAD around it:
> #if APR_HAS_THREADS
> SVN_ERR(svn_mutex__init(&ffsd->txn_list_lock, TRUE, common_pool));
> but that seems like unnecessary work. I would rather see this
> SVN_ERR(svn_mutex__init(&ffsd->txn_list_lock, common_pool));
> and the mutex will behave with or without locking automatically.
> Finally, looking at svn_mutex__init's implementation, I would state
> that the wrapped pointer will always either be NULL or a valid
> pointer, since you always set it to 0.
No, the reason behind the flag is its usage in APIs like
svn_cache__create_inprocess() where the caller can
decide whether a synchronization is necessary for a
given instance. If it won't be used from multiple threads,
the implementation will simply avoid the mutex overhead.
Received on 2011-07-04 23:33:45 CEST