On Tue, Aug 12, 2014 at 9:34 PM, Ivan Zhakov <i...@visualsvn.com> wrote:
> 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?
>
I can't think of a way to trigger these errors through our API.
So, you are probably right that this should be treated as a
malfunction. Changed in r1618600.
-- Stefan^2.
