From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 4/5] relay: close xlog cursor in relay thread Date: Sat, 29 Dec 2018 00:21:50 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: To: tarantool-patches@freelists.org List-ID: An xlog_cursor created and used by a relay via recovery context is destroyed by the main thread once the relay thread has exited. This is incorrect, because xlog_cursor uses cord's slab allocator and therefore must be destroyed in the same thread it was created by, otherwise we risk getting a use-after-free bug. So this patch moves recovery_delete() invocation to the end of the relay thread routine. No test is added, because our existing tests already cover this case - crashes don't usually happen, because we are lucky. The next patch will add some assertions to make the bug 100% reproducible. Closes #3910 --- src/box/relay.cc | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/box/relay.cc b/src/box/relay.cc index 988c01d3..8f5355ae 100644 --- a/src/box/relay.cc +++ b/src/box/relay.cc @@ -210,6 +210,26 @@ relay_cancel(struct relay *relay) } } +/** + * Called by a relay thread right before termination. + */ +static void +relay_exit(struct relay *relay) +{ + struct errinj *inj = errinj(ERRINJ_RELAY_EXIT_DELAY, ERRINJ_DOUBLE); + if (inj != NULL && inj->dparam > 0) + fiber_sleep(inj->dparam); + + /* + * Destroy the recovery context. We MUST do it in + * the relay thread, because it contains an xlog + * cursor, which must be closed in the same thread + * that opened it (it uses cord's slab allocator). + */ + recovery_delete(relay->r); + relay->r = NULL; +} + static void relay_stop(struct relay *relay) { @@ -277,6 +297,8 @@ int relay_final_join_f(va_list ap) { struct relay *relay = va_arg(ap, struct relay *); + auto guard = make_scoped_guard([=] { relay_exit(relay); }); + coio_enable(); relay_set_cord_name(relay->io.fd); @@ -601,10 +623,7 @@ relay_subscribe_f(va_list ap) NULL, NULL, cbus_process); cbus_endpoint_destroy(&relay->endpoint, cbus_process); - struct errinj *inj = errinj(ERRINJ_RELAY_EXIT_DELAY, ERRINJ_DOUBLE); - if (inj != NULL && inj->dparam > 0) - fiber_sleep(inj->dparam); - + relay_exit(relay); return -1; } -- 2.11.0