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.
-- Stefan^2.
