From 76847d9b4c27ec3011ef5866c3ea7b30fdcd2f08 Mon Sep 17 00:00:00 2001
From: Igor Nikonov <igor@clickhouse.com>
Date: Fri, 2 Aug 2024 22:43:05 +0000
Subject: [PATCH] Fix(asan) : access destroyed shared context from
 handleCrash()

---
 programs/server/Server.cpp   |  1 +
 src/Daemon/BaseDaemon.cpp    |  2 --
 src/Interpreters/Context.cpp | 27 +++++++++++++++++----------
 src/Interpreters/Context.h   |  8 ++++++--
 4 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/programs/server/Server.cpp b/programs/server/Server.cpp
index 20db4c2773c..5b9bb8b989d 100644
--- a/programs/server/Server.cpp
+++ b/programs/server/Server.cpp
@@ -977,6 +977,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.");
diff --git a/src/Daemon/BaseDaemon.cpp b/src/Daemon/BaseDaemon.cpp
index e7ae8ea5a1d..f74bd5e122c 100644
--- a/src/Daemon/BaseDaemon.cpp
+++ b/src/Daemon/BaseDaemon.cpp
@@ -23,9 +23,7 @@
 #include <unistd.h>
 
 #include <algorithm>
-#include <typeinfo>
 #include <iostream>
-#include <fstream>
 #include <memory>
 #include <base/scope_guard.h>
 
diff --git a/src/Interpreters/Context.cpp b/src/Interpreters/Context.cpp
index 5413b568068..743e762c81a 100644
--- a/src/Interpreters/Context.cpp
+++ b/src/Interpreters/Context.cpp
@@ -892,6 +892,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) {}
 
@@ -913,14 +919,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>());
@@ -2691,7 +2689,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();
 }
 
@@ -4084,8 +4086,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();
 }
diff --git a/src/Interpreters/Context.h b/src/Interpreters/Context.h
index d5e35c3e4b3..9ab7e9169c4 100644
--- a/src/Interpreters/Context.h
+++ b/src/Interpreters/Context.h
@@ -490,6 +490,8 @@ public:
 
     KitchenSink kitchen_sink;
 
+    void resetSharedContext();
+
 protected:
     using SampleBlockCache = std::unordered_map<std::string, Block>;
     mutable SampleBlockCache sample_block_cache;
@@ -527,6 +529,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.
@@ -1385,8 +1391,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);