Make GlobalState ref counted

Theoretically, this should prevent Context destructors causing segfaults if they're destructed after the GlobalState instance.
TileDB-Inc · Mar 11, 2025 · fc99837 · fc99837
1 parent 1c2f97a
commit fc99837
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 13 deletions.
diff --git a/tiledb/sm/global_state/global_state.cc b/tiledb/sm/global_state/global_state.cc
@@ -31,6 +31,7 @@
  */
 
 #include "tiledb/sm/global_state/global_state.h"
+#include "tiledb/common/dynamic_memory/dynamic_memory.h"
 #include "tiledb/sm/global_state/signal_handlers.h"
 #include "tiledb/sm/global_state/watchdog.h"
 #include "tiledb/sm/misc/constants.h"
@@ -47,9 +48,10 @@ using namespace tiledb::common;
 
 namespace tiledb::sm::global_state {
 
-GlobalState& GlobalState::GetGlobalState() {
+shared_ptr<GlobalState> GlobalState::GetGlobalState() {
   // This is thread-safe in C++11.
-  static GlobalState globalState;
+  static shared_ptr<GlobalState> globalState =
+      tdb::make_shared<GlobalState>(HERE());
   return globalState;
 }
 

diff --git a/tiledb/sm/global_state/global_state.h b/tiledb/sm/global_state/global_state.h
@@ -37,6 +37,7 @@
 #include <set>
 #include <string>
 
+#include "tiledb/common/common.h"
 #include "tiledb/sm/config/config.h"
 #include "tiledb/sm/storage_manager/storage_manager_declaration.h"
 
@@ -59,8 +60,11 @@ class GlobalState {
   GlobalState& operator=(const GlobalState&) = delete;
   GlobalState& operator=(const GlobalState&&) = delete;
 
+  /** Constructor. */
+  GlobalState();
+
   /** Returns a reference to the singleton GlobalState instance. */
-  static GlobalState& GetGlobalState();
+  static shared_ptr<GlobalState> GetGlobalState();
 
   /**
    * Initializes all TileDB global state in an idempotent and threadsafe way.
@@ -101,9 +105,6 @@ class GlobalState {
 
   /** Mutex protecting list of StorageManagers. */
   std::mutex storage_managers_mtx_;
-
-  /** Constructor. */
-  GlobalState();
 };
 
 }  // namespace global_state

diff --git a/tiledb/sm/global_state/watchdog.cc b/tiledb/sm/global_state/watchdog.cc
@@ -73,13 +73,15 @@ void Watchdog::watchdog_thread(Watchdog* watchdog) {
     return;
   }
 
+  auto global_state = GlobalState::GetGlobalState();
+
   while (true) {
     std::unique_lock<std::mutex> lck(watchdog->mtx_);
     watchdog->cv_.wait_for(
         lck, std::chrono::milliseconds(constants::watchdog_thread_sleep_ms));
 
     if (SignalHandlers::signal_received()) {
-      for (auto* sm : GlobalState::GetGlobalState().storage_managers()) {
+      for (auto* sm : global_state->storage_managers()) {
         throw_if_not_ok(sm->cancel_all_tasks());
       }
     }

diff --git a/tiledb/sm/global_state/watchdog.h b/tiledb/sm/global_state/watchdog.h
@@ -37,6 +37,8 @@
 #include <mutex>
 #include <thread>
 
+#include "tiledb/sm/global_state/global_state.h"
+
 namespace tiledb {
 namespace sm {
 namespace global_state {

diff --git a/tiledb/sm/storage_manager/storage_manager.cc b/tiledb/sm/storage_manager/storage_manager.cc
@@ -60,18 +60,17 @@ StorageManagerCanonical::StorageManagerCanonical(
     ContextResources& resources,
     const shared_ptr<Logger>&,  // unused
     const Config& config)
-    : vfs_(resources.vfs())
+    : global_state_(global_state::GlobalState::GetGlobalState())
+    , vfs_(resources.vfs())
     , cancellation_in_progress_(false)
     , config_(config)
     , queries_in_progress_(0) {
-  auto& global_state = global_state::GlobalState::GetGlobalState();
-  global_state.init(config_);
-
-  global_state.register_storage_manager(this);
+  global_state_->init(config_);
+  global_state_->register_storage_manager(this);
 }
 
 StorageManagerCanonical::~StorageManagerCanonical() {
-  global_state::GlobalState::GetGlobalState().unregister_storage_manager(this);
+  global_state_->unregister_storage_manager(this);
 
   throw_if_not_ok(cancel_all_tasks());
 

diff --git a/tiledb/sm/storage_manager/storage_manager_canonical.h b/tiledb/sm/storage_manager/storage_manager_canonical.h
@@ -52,6 +52,7 @@
 #include "tiledb/sm/array/array_directory.h"
 #include "tiledb/sm/enums/walk_order.h"
 #include "tiledb/sm/filesystem/uri.h"
+#include "tiledb/sm/global_state/global_state.h"
 #include "tiledb/sm/group/group.h"
 #include "tiledb/sm/misc/cancelable_tasks.h"
 #include "tiledb/sm/misc/types.h"
@@ -146,6 +147,9 @@ class StorageManagerCanonical {
   /*        PRIVATE ATTRIBUTES         */
   /* ********************************* */
 
+  /** The GlobalState to use for this StorageManager. */
+  shared_ptr<global_state::GlobalState> global_state_;
+
   /**
    * VFS instance used in `cancel_all_tasks`.
    */