maria-developers team mailing list archive

[Jonas Oreland <jonaso@xxxxxxxxxx>]

i think writing unit tests for these low-level primitives would be worth
while.
especially now in light of recent problems found...and the #define-madness
obfuscation

/Jonas

On Wed, Nov 26, 2014 at 9:32 AM, Sergey Vojtovich <svoj@xxxxxxxxxxx> wrote:

> Hi Kristian,
>
> On Mon, Nov 24, 2014 at 03:27:21PM +0100, Kristian Nielsen wrote:
> > Sergey Vojtovich <svoj@xxxxxxxxxxx> writes:
> >
> > > revno: 4369
> > > revision-id: svoj@xxxxxxxxxxx-20141120094823-fozdsrm1kzv46kxi
> > > parent: jplindst@xxxxxxxxxxx-20141119182734-cwbaed0ka90ocj5e
> > > committer: Sergey Vojtovich <svoj@xxxxxxxxxxx>
> > > branch nick: 5.5
> > > timestamp: Thu 2014-11-20 13:48:23 +0400
> > > message:
> > >   MDEV-7148 - Recurring: InnoDB: Failing assertion: !lock->recursive
> > >
> > >   On PPC64 high-loaded server may crash due to assertion failure in
> InnoDB
> > >   rwlocks code.
> > >
> > >   This happened because load order between "recursive" and
> "writer_thread"
> > >   wasn't properly enforced.
> >
> > Looks ok.
> >
> > I failed to understand why the patch would fix the assertion mentioned
> in the
> > bug report. But the patch itself looks correct.
> That's a bit tricky... I'll explain just one of the simplest side effects
> of
> this bug, which is half way to this assertion failure. Please let me know
> if
> you want me to track it down to the assertion failure.
>
> Initial
> -------
> lock->lock_word= X_LOCK_DECR // means unlocked
> lock->recursive= FALSE
> lock->writer_thread= 0
>
> thr1 acquires exclusive lock
> ----------------------------
> thr1: lock->lock_word-= X_LOCK_DECR
> @rw_lock_x_lock_low()/rw_lock_lock_word_decr()
> thr1: lock->writer_thread= thr1
> @rw_lock_x_lock_low()/rw_lock_set_writer_id_and_recursion_flag()
> thr1: sync
>  @rw_lock_x_lock_low()/rw_lock_set_writer_id_and_recursion_flag()
> thr1: lock->recursive= TRUE
> @rw_lock_x_lock_low()/rw_lock_set_writer_id_and_recursion_flag()
>
> thr1 releases exclusive lock
> ----------------------------
> thr1: if (lock->lock_word == 0)
>         lock->recursive = FALSE
> @rw_lock_x_unlock_func()/rw_lock_x_prepare_unlock()
> thr1: lock->lock_word+= X_LOCK_DECR
>
> Note: we only reset recursive, but writer_thread still equals to thr1.
>
> thr2 and thr1 contest for the lock
> ----------------------------------
> thr2: lock->lock_word-= X_LOCK_DECR
> @rw_lock_x_lock_low()/rw_lock_lock_word_decr()
> thr1: sees that X-lock is acquired,
>       it now needs to check if it is
>       relock or failed lock          @rw_lock_x_lock_low()
> thr1: loads lock->writer_thread,
>       which is still thr1            @rw_lock_x_lock_low()
> thr2: lock->writer_thread= thr1
> @rw_lock_x_lock_low()/rw_lock_set_writer_id_and_recursion_flag()
> thr2: sync
>  @rw_lock_x_lock_low()/rw_lock_set_writer_id_and_recursion_flag()
> thr2: lock->recursive= TRUE
> @rw_lock_x_lock_low()/rw_lock_set_writer_id_and_recursion_flag()
> thr1: loads lock->recursive, which
>       is TRUE                        @rw_lock_x_lock_low()
> thr1: if (lock->recursive && lock->writer_thread == thr1)
>         lock->lock_word -= X_LOCK_DECR  @rw_lock_x_lock_low()
>
> At this point we have 2 threads holding exclusive lock!
>
> Regards,
> Sergey
>
> _______________________________________________
> Mailing list: https://launchpad.net/~maria-developers
> Post to     : maria-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~maria-developers
> More help   : https://help.launchpad.net/ListHelp
>