Skip to content

Commit 19aaa11

Browse files
azatazat
committed
Fix deadlock on shutdown due to recursive context locking during library bridge cleanup
<details> <summary>stacktrace</summary> #5 DB::ContextSharedMutex::lockSharedImpl (this=this@entry=0x7ffff5b96040) at /src/ch/clickhouse/src/Interpreters/Context.cpp:1036 watch = {start_ns = 98678735300141, stop_ns = 0, clock_type = 4, is_running = true} increment = {what = <optimized out>, amount = 1} #6 0x00005555699352d6 in DB::SharedMutexHelper<DB::ContextSharedMutex, DB::SharedMutex>::lock_shared (this=0x7ffff5b96040) at /src/ch/clickhouse/src/Common/SharedMutexHelper.h:70 #7 DB::SharedLockGuard<DB::ContextSharedMutex>::SharedLockGuard (mutex_=..., this=<optimized out>) at /src/ch/clickhouse/src/Common/SharedLockGuard.h:19 #8 DB::ContextSharedPart::getConfigRef (this=0x7ffff5b96000) at /src/ch/clickhouse/src/Interpreters/Context.cpp:760 #9 DB::Context::getConfigRef (this=<optimized out>) at /src/ch/clickhouse/src/Interpreters/Context.cpp:1590 #10 0x000055556450941c in DB::ProxyConfigurationResolverProvider::get (request_protocol=DB::ProxyConfiguration::Protocol::HTTP) at /src/ch/clickhouse/src/Common/ProxyConfigurationResolverProvider.cpp:161 #11 0x00005555669ea75f in DB::BuilderRWBufferFromHTTP::create (this=0x7fffffffa848, credentials_=...) at /src/ch/clickhouse/src/IO/ReadWriteBufferFromHTTP.cpp:814 #12 0x000055556cea0035 in DB::ExternalDictionaryLibraryBridgeHelper::bridgeHandShake (this=0x7ffd60c4ca18) at /src/ch/clickhouse/src/BridgeHelper/ExternalDictionaryLibraryBridgeHelper.cpp:79 #13 0x000055556cea1f34 in DB::ExternalDictionaryLibraryBridgeHelper::removeLibrary (this=0x7ffff5b96044) at /src/ch/clickhouse/src/BridgeHelper/ExternalDictionaryLibraryBridgeHelper.cpp:186 #14 0x0000555566cb0248 in DB::LibraryDictionarySource::~LibraryDictionarySource (this=0x7ffd60c78318) at /src/ch/clickhouse/src/Dictionaries/LibraryDictionarySource.cpp:70 #15 0x000055556708d775 in std::__1::__shared_count::__release_shared[abi:ne190107]() (this=0x7ffd60c78300) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__memory/shared_ptr.h:156 #16 std::__1::__shared_weak_count::__release_shared[abi:ne190107]() (this=0x7ffd60c78300) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__memory/shared_ptr.h:185 #17 std::__1::shared_ptr<DB::IDictionarySource>::~shared_ptr[abi:ne190107]() (this=0x7ffd60c605b0) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__memory/shared_ptr.h:668 #18 DB::HashedDictionary<(DB::DictionaryKeyType)0, false, false>::~HashedDictionary (this=0x7ffd60c60398) at /src/ch/clickhouse/src/Dictionaries/HashedDictionary.h:378 #19 0x0000555569a9c477 in std::__1::__shared_count::__release_shared[abi:ne190107]() (this=0x7ffd60c60380) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__memory/shared_ptr.h:156 #20 std::__1::__shared_weak_count::__release_shared[abi:ne190107]() (this=0x7ffd60c60380) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__memory/shared_ptr.h:185 #21 std::__1::shared_ptr<DB::IExternalLoadable>::~shared_ptr[abi:ne190107]() (this=0x7ffff4832560) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__memory/shared_ptr.h:668 #22 DB::ExternalLoader::LoadingDispatcher::Info::~Info (this=0x7ffff4832548) at /src/ch/clickhouse/src/Interpreters/ExternalLoader.cpp:719 #23 std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const, DB::ExternalLoader::LoadingDispatcher::Info>::~pair (this=0x7ffff4832530, this@entry=0x7ffd60c60390) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__utility/pair.h:65 #24 __destroy_at<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const, DB::ExternalLoader::LoadingDispatcher::Info>, 0> (__loc=__loc@entry=0x7ffff4832530) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__memory/construct_at.h:67 #25 0x0000555569a9c628 in destroy<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const, DB::ExternalLoader::LoadingDispatcher::Info>, void, 0> (__p=0x7ffff5b96044, __p@entry=0x7ffff4832530) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__memory/allocator_traits.h:339 #28 std::__1::unique_ptr<std::__1::__hash_node<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DB::ExternalLoader::LoadingDispatcher::Info>, void*>, std::__1::__hash_node_destructor<std::__1::allocator<std::__1::__hash_node<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DB::ExternalLoader::LoadingDispatcher::Info>, void*> > > >::~unique_ptr[abi:ne190107]() (this=<optimized out>) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__memory/unique_ptr.h:261 ClickHouse#31 DB::ExternalLoader::LoadingDispatcher::setConfiguration (this=this@entry=0x7ffecad4c300, new_configs=...) at /src/ch/clickhouse/src/Interpreters/ExternalLoader.cpp:495 ClickHouse#32 0x0000555569a9116d in DB::ExternalLoader::reloadConfig (this=0x7ffecb3fd740, repository_name=...) at /src/ch/clickhouse/src/Interpreters/ExternalLoader.cpp:1533 ClickHouse#33 DB::ExternalLoader::addConfigRepository(std::__1::unique_ptr<DB::IExternalLoaderConfigRepository, std::__1::default_delete<DB::IExternalLoaderConfigRepository> >) const::$_0::operator()() const (this=<optimized out>) at /src/ch/clickhouse/src/Interpreters/ExternalLoader.cpp:1334 ClickHouse#34 std::__1::__invoke[abi:ne190107]<DB::ExternalLoader::addConfigRepository(std::__1::unique_ptr<DB::IExternalLoaderConfigRepository, std::__1::default_delete<DB::IExternalLoaderConfigRepository> >) const::$_0&>(DB::ExternalLoader::addConfigRepository(std::__1::unique_ptr<DB::IExternalLoaderConfigRepository, std::__1::default_delete<DB::IExternalLoaderConfigRepository> >) const::$_0&) (__f=...) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__type_traits/invoke.h:149 ClickHouse#35 std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:ne190107]<DB::ExternalLoader::addConfigRepository(std::__1::unique_ptr<DB::IExternalLoaderConfigRepository, std::__1::default_delete<DB::IExternalLoaderConfigRepository> >) const::$_0&>(DB::ExternalLoader::addConfigRepository(std::__1::unique_ptr<DB::IExternalLoaderConfigRepository, std::__1::default_delete<DB::IExternalLoaderConfigRepository> >) const::$_0&) (__args=...) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__type_traits/invoke.h:224 ClickHouse#36 std::__1::__function::__default_alloc_func<DB::ExternalLoader::addConfigRepository(std::__1::unique_ptr<DB::IExternalLoaderConfigRepository, std::__1::default_delete<DB::IExternalLoaderConfigRepository> >) const::$_0, void ()>::operator()[abi:ne190107]() (this=<optimized out>) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__functional/function.h:210 ClickHouse#37 std::__1::__function::__policy_invoker<void ()>::__call_impl[abi:ne190107]<std::__1::__function::__default_alloc_func<DB::ExternalLoader::addConfigRepository(std::__1::unique_ptr<DB::IExternalLoaderConfigRepository, std::__1::default_delete<DB::IExternalLoaderConfigRepository> >) const::$_0, void ()> ClickHouse#38 0x0000555569966aaa in std::__1::__function::__policy_func<void ()>::operator()[abi:ne190107]() const (this=0x7ffff5b96044) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__functional/function.h:716 ClickHouse#39 std::__1::function<void()>::operator() (this=0x7ffff5b96044) at /src/ch/clickhouse/contrib/llvm-project/libcxx/include/__functional/function.h:989 ClickHouse#40 BasicScopeGuard<std::__1::function<void()> >::invoke (this=0x7ffff5b96044) at /src/ch/clickhouse/base/base/../base/scope_guard.h:101 ClickHouse#41 BasicScopeGuard<std::__1::function<void()> >::reset (this=0x7ffff5b96044) at /src/ch/clickhouse/base/base/../base/scope_guard.h:63 ClickHouse#42 DB::ContextSharedPart::shutdown (this=0x7ffff5b96000) at /src/ch/clickhouse/src/Interpreters/Context.cpp:909 ClickHouse#43 0x00005555644cf5cb in DB::Server::main(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&)::$_3::operator()() const (this=this@entry=0x7fffffffc870) at /src/ch/clickhouse/programs/server/Server.cpp:1257 ClickHouse#44 0x00005555644b4439 in BasicScopeGuard<DB::Server::main(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&)::$_3>::invoke (this=0x7fffffffc870) at /src/ch/clickhouse/base/base/../base/scope_guard.h:101 ClickHouse#45 BasicScopeGuard<DB::Server::main(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&)::$_3>::~BasicScopeGuard (this=0x7fffffffc870) at /src/ch/clickhouse/base/base/../base/scope_guard.h:50 ClickHouse#46 DB::Server::main (this=0x7fffffffe4c8) at /src/ch/clickhouse/programs/server/Server.cpp:2788 ClickHouse#47 0x0000555572cbbee9 in Poco::Util::Application::run (this=0x7fffffffe4c8) at /src/ch/clickhouse/base/poco/Util/src/Application.cpp:315 ClickHouse#48 0x000055556449c390 in DB::Server::run (this=0x7fffffffe4c8) at /src/ch/clickhouse/programs/server/Server.cpp:610 ClickHouse#49 0x0000555564499aec in mainEntryClickHouseServer (argc=6, argv=0x7ffff67ecdc0) at /src/ch/clickhouse/programs/server/Server.cpp:397 ClickHouse#50 0x000055555dc994c2 in main (argc_=<optimized out>, argv_=<optimized out>) at /src/ch/clickhouse/programs/main.cpp:340 </details>
1 parent 9e61fdf commit 19aaa11

File tree

1 file changed

+17
-11
lines changed

1 file changed

+17
-11
lines changed

‎src/Interpreters/Context.cpp‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -836,6 +836,9 @@ struct ContextSharedPart : boost::noncopyable
836836
std::unique_ptr<DDLWorker> delete_ddl_worker;
837837
std::unique_ptr<AccessControl> delete_access_control;
838838

839+
scope_guard delete_dictionaries_xmls;
840+
scope_guard delete_user_defined_executable_functions_xmls;
841+
839842
/// Delete DDLWorker before zookeeper.
840843
/// Cause it can call Context::getZooKeeper and resurrect it.
841844

@@ -907,17 +910,8 @@ struct ContextSharedPart : boost::noncopyable
907910
/// Preemptive destruction is important, because these objects may have a refcount to ContextShared (cyclic reference).
908911
/// TODO: Get rid of this.
909912

910-
/// Dictionaries may be required:
911-
/// - for storage shutdown (during final flush of the Buffer engine)
912-
/// - before storage startup (because of some streaming of, i.e. Kafka, to
913-
/// the table with materialized column that has dictGet)
914-
///
915-
/// So they should be created before any storages and preserved until storages will be terminated.
916-
///
917-
/// But they cannot be created before storages since they may required table as a source,
918-
/// but at least they can be preserved for storage termination.
919-
dictionaries_xmls.reset();
920-
user_defined_executable_functions_xmls.reset();
913+
delete_dictionaries_xmls = std::move(dictionaries_xmls);
914+
delete_user_defined_executable_functions_xmls = std::move(user_defined_executable_functions_xmls);
921915

922916
delete_system_logs = std::move(system_logs);
923917
delete_embedded_dictionaries = std::move(embedded_dictionaries);
@@ -939,6 +933,18 @@ struct ContextSharedPart : boost::noncopyable
939933
zookeeper.reset();
940934
}
941935

936+
/// Dictionaries may be required:
937+
/// - for storage shutdown (during final flush of the Buffer engine)
938+
/// - before storage startup (because of some streaming of, i.e. Kafka, to
939+
/// the table with materialized column that has dictGet)
940+
///
941+
/// So they should be created before any storages and preserved until storages will be terminated.
942+
///
943+
/// But they cannot be created before storages since they may required table as a source,
944+
/// but at least they can be preserved for storage termination.
945+
delete_dictionaries_xmls.reset();
946+
delete_user_defined_executable_functions_xmls.reset();
947+
942948
/// Can be removed without context lock
943949
delete_system_logs.reset();
944950
delete_embedded_dictionaries.reset();

0 commit comments

Comments
 (0)