From 241898af7eb5b23676ecc0d24d8c0d159b1a8e64 Mon Sep 17 00:00:00 2001 From: Teemu Ollakka Date: Thu, 2 Mar 2023 08:38:19 +0200 Subject: [PATCH] Postpone streaming_context cleanup into after_rollback() Streaming_context was cleaned up in streaming_rollback(), which could cause clearing fragment seqno vector while it was still accessed by the owning thread, causing undefined behavior. Fixed by postponing streaming_context cleanup for BF aborted SR transactions to always happen in after_rollback(). --- src/transaction.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/transaction.cpp b/src/transaction.cpp index f050a1a..b28ef5a 100644 --- a/src/transaction.cpp +++ b/src/transaction.cpp @@ -1961,12 +1961,19 @@ void wsrep::transaction::streaming_rollback( if (streaming_context_.rolled_back() == false) { + // Note that streaming_context_ must not be cleaned up in this + // method. This is because the owning thread may still be executing + // fragment removal on commit, which will access fragment + // vector in streaming context. Clearing streaming context + // here may cause owning thread to access memory which was + // already freed. Cleanup for streaming_context_ will happen + // in after_rollback(). + if (bf_aborted_in_total_order_) { lock.unlock(); server_service_.debug_sync("wsrep_streaming_rollback"); client_state_.server_state_.stop_streaming_client(&client_state_); - // Fragments are removed in after_rollback(). lock.lock(); } else @@ -1980,7 +1987,6 @@ void wsrep::transaction::streaming_rollback( client_state_.server_state_.convert_streaming_client_to_applier( &client_state_); lock.lock(); - streaming_context_.cleanup(); enum wsrep::provider::status status(provider().rollback(id_)); if (status)