On 02.07.2011 04:35, Blair Zajac wrote:
On Jul 1, 2011, at 5:27 PM, stef...@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
Log:
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,
common_pool);
- if (status)
- return svn_error_wrap_apr(status,
- _("Can't create FSFS txn list
mutex"));
-#endif
-
+ SVN_ERR(svn_mutex__init(&ffsd->txn_list_lock, TRUE,
common_pool));
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));
#endif
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.
-- Stefan^2.
