On 04.08.2014 15:22, Ivan Zhakov wrote:
> On 2 August 2014 23:13, <stef...@apache.org> wrote:
> @@ -195,7 +206,9 @@ svn_mutex__unlock(svn_mutex__t *mutex,
> #else
> /* Update lock counter. */
> if (mutex->checked)
> - --mutex->count;
> + if (--mutex->count)
> + return svn_error_create(SVN_ERR_INVALID_UNLOCK, NULL,
> + _("Tried to release a non-locked mutex"));
> 1. Why you are using non-atomic decrement here while we're using
> casptr() for mutex->owner?
This bit of code is only used when APR does *not* support threads, IIRC.
So atomic insns are overkill.
> 2. You're leaving mutex object in invalid state on attempt to release
> non-locked mutex.
That's been fixed.
> I also find checked mutexes a 'false sense of security' thing: we
> should be more careful when writing multi-threaded code instead of
> relying on unreliable protection mechanism. That is because many
> multi-threading problems do not have stable reproduction scripts
I feel the same about that ... I'd have recommended to just use the
APR_THREAD_MUTEX_UNNESTED flag, but then on Windows, this uses an event
instead of a critical section.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. br...@wandisco.com
