Backport #82233 to 25.6: Fix possible data-race between suggestion thread and main client thread

robot-clickhouse · robot-clickhouse · commit 41532f5ef7a8 · 2025-06-20T15:10:30.000Z
diff --git a/src/Client/LineReader.cpp b/src/Client/LineReader.cpp
@@ -79,18 +79,20 @@ replxx::Replxx::completions_t LineReader::Suggest::getCompletions(const String &
 
     std::pair<Words::const_iterator, Words::const_iterator> range;
 
-    std::lock_guard lock(mutex);
-
     Words to_search;
     bool no_case = false;
-    /// Only perform case sensitive completion when the prefix string contains any uppercase characters
-    if (std::none_of(prefix.begin(), prefix.end(), [](char32_t x) { return iswupper(static_cast<wint_t>(x)); }))
+
     {
-        to_search = words_no_case;
-        no_case = true;
+        std::lock_guard lock(mutex);
+        /// Only perform case sensitive completion when the prefix string contains any uppercase characters
+        if (std::none_of(prefix.begin(), prefix.end(), [](char32_t x) { return iswupper(static_cast<wint_t>(x)); }))
+        {
+            to_search = words_no_case;
+            no_case = true;
+        }
+        else
+            to_search = words;
     }
-    else
-        to_search = words;
 
     if (custom_completions_callback)
     {
diff --git a/src/Client/Suggest.cpp b/src/Client/Suggest.cpp
@@ -4,6 +4,7 @@
 #include <AggregateFunctions/Combinators/AggregateFunctionCombinatorFactory.h>
 #include <Columns/ColumnString.h>
 #include <Common/Exception.h>
+#include <Common/setThreadName.h>
 #include <Common/typeid_cast.h>
 #include <Common/Macros.h>
 #include "Core/Protocol.h"
@@ -94,7 +95,15 @@ void Suggest::load(ContextPtr context, const ConnectionParameters & connection_p
 {
     loading_thread = std::thread([my_context=Context::createCopy(context), connection_parameters, suggestion_limit, this]
     {
+        /// Creates new QueryScope/ThreadStatus to avoid sharing global context, which settings can be modified by the client in another thread.
         ThreadStatus thread_status;
+        std::optional<CurrentThread::QueryScope> query_scope;
+        /// LocalConnection creates QueryScope for each query
+        if constexpr (!std::is_same_v<ConnectionType, LocalConnection>)
+            query_scope.emplace(my_context);
+
+        setThreadName("Suggest");
+
         for (size_t retry = 0; retry < 10; ++retry)
         {
             try
diff --git a/tests/queries/0_stateless/02160_client_autocomplete_parse_query.expect b/tests/queries/0_stateless/02160_client_autocomplete_parse_query.expect
@@ -11,9 +11,7 @@ exp_internal -f $CLICKHOUSE_TMP/$basename.debuglog 0
 set history_file $CLICKHOUSE_TMP/$basename.history
 
 log_user 0
-# FIXME: workaround to obtain the stacktrace of the client in case of timeout
-# (clickhouse-test wrapper does this on timeout)
-set timeout 3600
+set timeout 60
 set uuid ""
 match_max 100000
 expect_after {
diff --git a/tests/queries/0_stateless/02815_no_throw_in_simple_queries.sh b/tests/queries/0_stateless/02815_no_throw_in_simple_queries.sh
@@ -31,7 +31,13 @@ log_user 0
 set timeout 60
 match_max 100000
 
-spawn bash -c "$command"
+exp_internal -f $CLICKHOUSE_TMP/$(basename "${BASH_SOURCE[0]}").debuglog 0
+expect_after {
+    -i \$any_spawn_id eof { exp_continue }
+    -i \$any_spawn_id timeout { exit 1 }
+}
+
+spawn $command
 
 expect ":) "
 
