Tarantool development patches archive
 help / color / mirror / Atom feed
From: Serge Petrenko <sergepetrenko@tarantool.org>
To: sergeyb@tarantool.org, gorcunov@gmail.com
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH 2/2] relay: fix segfault on replica transition from anonymous
Date: Sat, 11 Apr 2020 15:33:45 +0300	[thread overview]
Message-ID: <867406d860b296af9373b7d3e32a72ee1d9270ac.1586606685.git.sergepetrenko@tarantool.org> (raw)
In-Reply-To: <cover.1586606685.git.sergepetrenko@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)

      parent reply	other threads:[~2020-04-11 12:34 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-11 12:33 [Tarantool-patches] [PATCH 0/2] fix replication/anon test segfaut Serge Petrenko
2020-04-11 12:33 ` [Tarantool-patches] [PATCH 1/2] trigger: add trigger_is_set() method Serge Petrenko
2020-04-11 16:33   ` Cyrill Gorcunov
2020-04-13  9:53     ` Serge Petrenko
2020-04-13 10:00       ` Cyrill Gorcunov
2020-04-11 12:33 ` Serge Petrenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=867406d860b296af9373b7d3e32a72ee1d9270ac.1586606685.git.sergepetrenko@tarantool.org \
    --to=sergepetrenko@tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=sergeyb@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 2/2] relay: fix segfault on replica transition from anonymous' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox