Merge pull request #34988 from azat/safe-exit

Fix signal-unsafe TSan report in client

base/base/CMakeLists.txt

@@ -17,6 +17,7 @@ set (SRCS
     terminalColors.cpp
     errnoToString.cpp
     StringRef.cpp
+    safeExit.cpp
 )
 
 if (ENABLE_REPLXX)

base/base/safeExit.cpp

@@ -0,0 +1,18 @@
+#if defined(OS_LINUX)
+#    include <sys/syscall.h>
+#endif
+#include <unistd.h>
+#include <base/safeExit.h>
+#include <base/defines.h>
+
+[[noreturn]] void safeExit(int code)
+{
+#if defined(THREAD_SANITIZER) && defined(OS_LINUX)
+    /// Thread sanitizer tries to do something on exit that we don't need if we want to exit immediately,
+    /// while connection handling threads are still run.
+    (void)syscall(SYS_exit_group, code);
+    __builtin_unreachable();
+#else
+    _exit(code);
+#endif
+}

base/base/safeExit.h

@@ -0,0 +1,4 @@
+#pragma once
+
+/// _exit() with a workaround for TSan.
+[[noreturn]] void safeExit(int code);

programs/keeper/Keeper.cpp

@@ -13,6 +13,7 @@
 #include <base/logger_useful.h>
 #include <base/ErrorHandlers.h>
 #include <base/scope_guard.h>
+#include <base/safeExit.h>
 #include <Poco/Net/NetException.h>
 #include <Poco/Net/TCPServerParams.h>
 #include <Poco/Net/TCPServer.h>
@@ -34,11 +35,6 @@
 #include <Server/ProtocolServerAdapter.h>
 #include <Server/KeeperTCPHandlerFactory.h>
 
-#if defined(OS_LINUX)
-#    include <unistd.h>
-#    include <sys/syscall.h>
-#endif
-
 
 int mainEntryClickHouseKeeper(int argc, char ** argv)
 {
@@ -127,18 +123,6 @@ Poco::Net::SocketAddress makeSocketAddress(const std::string & host, UInt16 port
     return socket_address;
 }
 
-[[noreturn]] void forceShutdown()
-{
-#if defined(THREAD_SANITIZER) && defined(OS_LINUX)
-    /// Thread sanitizer tries to do something on exit that we don't need if we want to exit immediately,
-    /// while connection handling threads are still run.
-    (void)syscall(SYS_exit_group, 0);
-    __builtin_unreachable();
-#else
-    _exit(0);
-#endif
-}
-
 std::string getUserName(uid_t user_id)
 {
     /// Try to convert user id into user name.
@@ -474,7 +458,7 @@ int Keeper::main(const std::vector<std::string> & /*args*/)
         if (current_connections)
         {
             LOG_INFO(log, "Will shutdown forcefully.");
-            forceShutdown();
+            safeExit(0);
         }
     });
 

programs/server/Server.cpp

@@ -22,6 +22,8 @@
 #include <base/getMemoryAmount.h>
 #include <base/errnoToString.h>
 #include <base/coverage.h>
+#include <base/getFQDNOrHostName.h>
+#include <base/safeExit.h>
 #include <Common/MemoryTracker.h>
 #include <Common/ClickHouseRevision.h>
 #include <Common/DNSResolver.h>
@@ -31,7 +33,6 @@
 #include <Common/StringUtils/StringUtils.h>
 #include <Common/ZooKeeper/ZooKeeper.h>
 #include <Common/ZooKeeper/ZooKeeperNodeCache.h>
-#include <base/getFQDNOrHostName.h>
 #include <Common/getMultipleKeysFromConfig.h>
 #include <Common/getNumberOfPhysicalCPUCores.h>
 #include <Common/getExecutablePath.h>
@@ -95,8 +96,6 @@
 #    include <sys/mman.h>
 #    include <sys/ptrace.h>
 #    include <Common/hasLinuxCapability.h>
-#    include <unistd.h>
-#    include <sys/syscall.h>
 #endif
 
 #if USE_SSL
@@ -505,19 +504,6 @@ void checkForUsersNotInMainConfig(
     }
 }
 
-[[noreturn]] void forceShutdown()
-{
-#if defined(THREAD_SANITIZER) && defined(OS_LINUX)
-    /// Thread sanitizer tries to do something on exit that we don't need if we want to exit immediately,
-    /// while connection handling threads are still run.
-    (void)syscall(SYS_exit_group, 0);
-    __builtin_unreachable();
-#else
-    _exit(0);
-#endif
-}
-
-
 int Server::main(const std::vector<std::string> & /*args*/)
 {
     Poco::Logger * log = &logger();
@@ -1527,7 +1513,7 @@ if (ThreadFuzzer::instance().isEffective())
                 /// Dump coverage here, because std::atexit callback would not be called.
                 dumpCoverageReportIfPossible();
                 LOG_INFO(log, "Will shutdown forcefully.");
-                forceShutdown();
+                safeExit(0);
             }
         });
 

src/Client/ClientBase.cpp

@@ -7,12 +7,13 @@
 #include <map>
 #include <unordered_map>
 
-#include <base/argsToConfig.h>
 #include <Common/DateLUT.h>
 #include <Common/LocalDate.h>
 #include <Common/MemoryTracker.h>
+#include <base/argsToConfig.h>
 #include <base/LineReader.h>
 #include <base/scope_guard_safe.h>
+#include <base/safeExit.h>
 #include <Common/Exception.h>
 #include <Common/getNumberOfPhysicalCPUCores.h>
 #include <Common/tests/gtest_global_context.h>
@@ -233,7 +234,7 @@ public:
 void interruptSignalHandler(int signum)
 {
     if (exit_on_signal.test_and_set())
-        _exit(128 + signum);
+        safeExit(128 + signum);
 }