From e6f5c2ba1a9d248f42801bd76ee8a29c67e35629 Mon Sep 17 00:00:00 2001
From: Alexey Milovidov <milovidov@clickhouse.com>
Date: Sat, 13 Jun 2026 23:56:56 +0000
Subject: [PATCH] Fix data race on peer config_ in handle_rpc_result

The `peer::config_` member is protected by `config_mutex_` and is meant
to be read only through the locked accessors (`get_id`, `get_endpoint`,
`get_config`, ...). Several call sites in `peer.cxx` still read
`config_->get_id` directly, bypassing the mutex.

ThreadSanitizer reports a data race between the unlocked read of
`srv_config::get_id` in `peer::handle_rpc_result` (an asio RPC worker
thread) and the construction of a fresh `srv_config` that is published
via `peer::set_config` while `cluster_config::deserialize` runs inside
`raft_server::commit_conf` on the commit thread. The lock on
`config_mutex_` is the intended synchronization point, so the unlocked
read leaves no happens-before edge.

Route all of these reads through the locked `get_id` accessor, matching
the earlier fix in commit 7a9de82 ("Fix some non dangerous races").
---
 src/peer.cxx | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/peer.cxx b/src/peer.cxx
index 52d1a6cf..8cbce25f 100644
--- a/src/peer.cxx
+++ b/src/peer.cxx
@@ -130,7 +130,7 @@ void peer::handle_rpc_result( ptr<peer> myself,
                           "current id %" PRIu64 ", from id %" PRIu64 ". "
                           "will ignore this response. "
                           "Currently, stale_resps: %d, response_limit: %d",
-                          config_->get_id(),
+                          get_id(),
                           cur_rpc_id,
                           my_rpc_client_id,
                           stale_resps,
@@ -139,7 +139,7 @@ void peer::handle_rpc_result( ptr<peer> myself,
                     p_wn( "[EDGE CASE] too verbose stale RPC response from peer %d, "
                           "will suppress it from now."
                           "Currently, stale_resps: %d, response_limit: %d",
-                          config_->get_id(),
+                          get_id(),
                           stale_resps,
                           limit );
                 }
@@ -189,7 +189,7 @@ void peer::handle_rpc_result( ptr<peer> myself,
                 reset_stream();
                 if (last_streamed_log_idx) {
                     p_in("stop stream mode for peer %d at idx: %" PRIu64 "",
-                         config_->get_id(), last_streamed_log_idx);
+                         get_id(), last_streamed_log_idx);
                 }
                 reset_stale_rpc_responses();
                 reset_bytes_in_flight();
@@ -217,7 +217,7 @@ void peer::handle_rpc_result( ptr<peer> myself,
                           "returning error: current id %" PRIu64
                           ", from id %" PRIu64 ". "
                           "Currently, stale_resps: %d, response_limit: %d",
-                          config_->get_id(),
+                          get_id(),
                           cur_rpc_id,
                           my_rpc_client_id,
                           stale_resps,
@@ -226,7 +226,7 @@ void peer::handle_rpc_result( ptr<peer> myself,
                     p_wn( "[EDGE CASE] too verbose stale RPC response from peer %d, "
                           "will suppress it from now."
                           "Currently, stale_resps: %d, response_limit: %d",
-                          config_->get_id(),
+                          get_id(),
                           stale_resps,
                           limit );
                 }
@@ -337,7 +337,7 @@ void peer::reopen(context& ctx, timer_task<int32>::executor& hb_exec) {
     hb_task_ = cs_new< timer_task<int32>,
                             timer_task<int32>::executor&,
                             int32 >
-                          ( hb_exec, config_->get_id(),
+                          ( hb_exec, get_id(),
                             timer_task_type::heartbeat_timer ) ;
     p_tr("call peer %d reopen succeeded", get_id());
 }