[Tarantool-patches] [PATCH v2] relay: fix segfault on replica transition from anonymous
Sergey Bronnikov
sergeyb at tarantool.org
Mon Apr 13 16:24:16 MSK 2020
Hi,
thanks for the fix!
On 13:37 Mon 13 Apr , Serge Petrenko wrote:
> Sergey, would you kindly check this patch with your reproducer?
> I failed to reproduce the issue on my side.
reproducer passed 200 times out of 200 runs.
> --
> Serge Petrenko
> sergepetrenko at tarantool.org
>
> > 13 апр. 2020 г., в 13:34, Serge Petrenko <sergepetrenko at tarantool.org> написал(а):
> >
> > relay_subscribe_f sets a recovery trigger notifying tx when a full log
> > is read and gc consumer corresponding to the replica may be advanced.
> > Since anonymous replicas do not have gc consumers, the trigger isn't
> > added for them. However, on relay exit, the trigger deletion depends
> > on replica->anon flag. This is buggy in case relay stalls on exit due to
> > replica disconnect. Replica has time to reconnect and register as a
> > normal instance, hence its replica->anon flag will be false by the time
> > we check whether to clear triggers or not, effectively making us to
> > clear unset triggers and segfault.
> >
> > Fix this by initializing the triggers with trigger_create(), which
> > allows a trigger_clear() call, even if the triggers are not set, and
> > omit the replica->anon check.
> >
> > Closes #4731
> >
> > Acked-by: Cyrill Gorcunov <gorcunov at gmail.com>
> > ---
> > https://github.com/tarantool/tarantool/tree/sp/gh-4731-anon-segfault-v2
> > https://github.com/tarantool/tarantool/issues/4731
> >
> > src/box/relay.cc | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/box/relay.cc b/src/box/relay.cc
> > index c634348a4..5f34753fc 100644
> > --- a/src/box/relay.cc
> > +++ b/src/box/relay.cc
> > @@ -580,9 +580,8 @@ relay_subscribe_f(va_list ap)
> > * Not needed for anonymous replicas, since they
> > * aren't registered with gc at all.
> > */
> > - struct trigger on_close_log = {
> > - RLIST_LINK_INITIALIZER, relay_on_close_log_f, relay, NULL
> > - };
> > + struct trigger on_close_log;
> > + trigger_create(&on_close_log, relay_on_close_log_f, relay, NULL);
> > if (!relay->replica->anon)
> > trigger_add(&r->on_close_log, &on_close_log);
> >
> > @@ -662,9 +661,12 @@ relay_subscribe_f(va_list ap)
> > diag_log();
> > say_crit("exiting the relay loop");
> >
> > - /* Clear garbage collector trigger and WAL watcher. */
> > - if (!relay->replica->anon)
> > - trigger_clear(&on_close_log);
> > + /*
> > + * Clear garbage collector trigger and WAL watcher.
> > + * trigger_clear() does nothing in case the triggers
> > + * aren't set (the replica is anonymous).
> > + */
> > + trigger_clear(&on_close_log);
> > wal_clear_watcher(&relay->wal_watcher, cbus_process);
> >
> > /* Join ack reader fiber. */
> > --
> > 2.21.1 (Apple Git-122.3)
> >
>
--
sergeyb@
More information about the Tarantool-patches
mailing list