From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (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 EC5884696C3 for ; Sat, 11 Apr 2020 15:34:08 +0300 (MSK) From: Serge Petrenko Date: Sat, 11 Apr 2020 15:33:45 +0300 Message-Id: <867406d860b296af9373b7d3e32a72ee1d9270ac.1586606685.git.sergepetrenko@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 2/2] relay: fix segfault on replica transition from anonymous List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: sergeyb@tarantool.org, gorcunov@gmail.com Cc: tarantool-patches@dev.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 makinig us to clear unset triggers and segfault. Fix this by actually checking whether the trigger is set instead of relying on replica->anon flag. Closes #4731 --- src/box/relay.cc | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/box/relay.cc b/src/box/relay.cc index c634348a4..5f6edcd78 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,8 +661,15 @@ 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) + /* + * Clear garbage collector trigger and WAL watcher. + * Note, even though we set the trigger only when the + * replica is anonymous, we cannot check replica->anon + * here to determine whether the trigger is set. The + * replica may have registered and become non-anonymous + * while this relay thread was still exiting. + */ + if (trigger_is_set(&on_close_log)) trigger_clear(&on_close_log); wal_clear_watcher(&relay->wal_watcher, cbus_process); -- 2.21.1 (Apple Git-122.3)