From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp41.i.mail.ru (smtp41.i.mail.ru [94.100.177.101]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id E07CB4696C3 for ; Mon, 13 Apr 2020 13:37:35 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\)) From: Serge Petrenko In-Reply-To: <20200413103454.82207-1-sergepetrenko@tarantool.org> Date: Mon, 13 Apr 2020 13:37:34 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20200413103454.82207-1-sergepetrenko@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2] relay: fix segfault on replica transition from anonymous List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Bronnikov Cc: tml Sergey, would you kindly check this patch with your reproducer? I failed to reproduce the issue on my side. -- Serge Petrenko sergepetrenko@tarantool.org > 13 =D0=B0=D0=BF=D1=80. 2020 =D0=B3., =D0=B2 13:34, Serge Petrenko = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0= =B0): >=20 > 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. >=20 > 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. >=20 > Closes #4731 >=20 > Acked-by: Cyrill Gorcunov > --- > = https://github.com/tarantool/tarantool/tree/sp/gh-4731-anon-segfault-v2 > https://github.com/tarantool/tarantool/issues/4731 >=20 > src/box/relay.cc | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) >=20 > 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 =3D { > - 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); >=20 > @@ -662,9 +661,12 @@ relay_subscribe_f(va_list ap) > diag_log(); > say_crit("exiting the relay loop"); >=20 > - /* 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); >=20 > /* Join ack reader fiber. */ > --=20 > 2.21.1 (Apple Git-122.3) >=20