On 11 August 2014 20:29, Stefan Fuhrmann <stefan.fuhrm...@wandisco.com> wrote:
>
> On Mon, Aug 4, 2014 at 4:23 PM, Ivan Zhakov <i...@visualsvn.com> wrote:
>>
>> On 4 August 2014 17:48, Branko Čibej <br...@wandisco.com> wrote:
>> > 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.
>> >
>> Ok. It was not clear from unidiff and log message that change is about
>> !APR_HAS_THREADS code.
>>
>> >
>> > 2. You're leaving mutex object in invalid state on attempt to release
>> > non-locked mutex.
>> >
>> >
>> > That's been fixed.
>> >
>> OK. Another question: why this situation not considered as malfunction?
>
>
> Because it can be triggered through (erroneous) use of our FS API.
>
> For instance: svn_fs_freeze(modify_stuff) will deadlock in 1.8 and
> give a "recursive lock" error in 1.9 Returning a malfunction would be
> misleading as there is nothing wrong with the SVN libs.
>
This is reason for check that lock is already obtained by the same
thread, while I'm asking why attempt to release non-locked mutex is
not considered as malfunction?
--
Ivan Zhakov
