Merge pull request #67700 from ClickHouse/asan-invalid-shared-context-access

Fix(asan): access destroyed shared context from handleCrash()

programs/server/Server.cpp

@@ -978,6 +978,7 @@ try
         /** Explicitly destroy Context. It is more convenient than in destructor of Server, because logger is still available.
           * At this moment, no one could own shared part of Context.
           */
+        global_context->resetSharedContext();
         global_context.reset();
         shared_context.reset();
         LOG_DEBUG(log, "Destroyed global context.");

src/Daemon/BaseDaemon.cpp

@@ -22,7 +22,6 @@
 #include <cstring>
 #include <unistd.h>
 #include <algorithm>
-#include <typeinfo>
 #include <iostream>
 #include <memory>
 

src/Interpreters/Context.cpp

@@ -893,6 +893,12 @@ ContextData::ContextData(const ContextData &o) :
 {
 }
 
+void ContextData::resetSharedContext()
+{
+    std::lock_guard<std::mutex> lock(mutex_shared_context);
+    shared = nullptr;
+}
+
 Context::Context() = default;
 Context::Context(const Context & rhs) : ContextData(rhs), std::enable_shared_from_this<Context>(rhs) {}
 
@@ -914,14 +920,6 @@ ContextMutablePtr Context::createGlobal(ContextSharedPart * shared_part)
     return res;
 }
 
-void Context::initGlobal()
-{
-    assert(!global_context_instance);
-    global_context_instance = shared_from_this();
-    DatabaseCatalog::init(shared_from_this());
-    EventNotifier::init();
-}
-
 SharedContextHolder Context::createShared()
 {
     return SharedContextHolder(std::make_unique<ContextSharedPart>());
@@ -2692,7 +2690,11 @@ void Context::makeSessionContext()
 
 void Context::makeGlobalContext()
 {
-    initGlobal();
+    assert(!global_context_instance);
+    global_context_instance = shared_from_this();
+    DatabaseCatalog::init(shared_from_this());
+    EventNotifier::init();
+
     global_context = shared_from_this();
 }
 
@@ -4088,8 +4090,13 @@ void Context::initializeTraceCollector()
 }
 
 /// Call after unexpected crash happen.
-void Context::handleCrash() const TSA_NO_THREAD_SAFETY_ANALYSIS
+void Context::handleCrash() const
 {
+    std::lock_guard<std::mutex> lock(mutex_shared_context);
+    if (!shared)
+        return;
+
+    SharedLockGuard lock2(shared->mutex);
     if (shared->system_logs)
         shared->system_logs->handleCrash();
 }

src/Interpreters/Context.h

@@ -492,6 +492,8 @@ public:
 
     KitchenSink kitchen_sink;
 
+    void resetSharedContext();
+
 protected:
     using SampleBlockCache = std::unordered_map<std::string, Block>;
     mutable SampleBlockCache sample_block_cache;
@@ -529,6 +531,10 @@ protected:
     mutable ThrottlerPtr local_write_query_throttler;       /// A query-wide throttler for local IO writes
 
     mutable ThrottlerPtr backups_query_throttler;           /// A query-wide throttler for BACKUPs
+
+    mutable std::mutex mutex_shared_context;    /// mutex to avoid accessing destroyed shared context pointer
+                                                /// some Context methods can be called after the shared context is destroyed
+                                                /// example, Context::handleCrash() method - called from signal handler
 };
 
 /** A set of known objects that can be used in the query.
@@ -1387,8 +1393,6 @@ private:
 
     ExternalUserDefinedExecutableFunctionsLoader & getExternalUserDefinedExecutableFunctionsLoaderWithLock(const std::lock_guard<std::mutex> & lock);
 
-    void initGlobal();
-
     void setUserID(const UUID & user_id_);
     void setCurrentRolesImpl(const std::vector<UUID> & new_current_roles, bool throw_if_not_granted, bool skip_if_not_granted, const std::shared_ptr<const User> & user);