Fix crash on destroying AccessControl: add explicit shutdown.

src/Access/AccessControl.cpp

@@ -261,7 +261,24 @@ AccessControl::AccessControl()
 }
 
 
-AccessControl::~AccessControl() = default;
+AccessControl::~AccessControl()
+{
+    try
+    {
+        shutdown();
+    }
+    catch (...)
+    {
+        tryLogCurrentException(__PRETTY_FUNCTION__);
+    }
+}
+
+
+void AccessControl::shutdown()
+{
+    MultipleAccessStorage::shutdown();
+    removeAllStorages();
+}
 
 
 void AccessControl::setUpFromMainConfig(const Poco::Util::AbstractConfiguration & config_, const String & config_path_,

src/Access/AccessControl.h

@@ -53,6 +53,9 @@ public:
     AccessControl();
     ~AccessControl() override;
 
+    /// Shutdown the access control and stops all background activity.
+    void shutdown() override;
+
     /// Initializes access storage (user directories).
     void setUpFromMainConfig(const Poco::Util::AbstractConfiguration & config_, const String & config_path_,
                              const zkutil::GetZooKeeper & get_zookeeper_function_);

src/Access/DiskAccessStorage.cpp

@@ -194,11 +194,9 @@ DiskAccessStorage::DiskAccessStorage(const String & storage_name_, const String
 
 DiskAccessStorage::~DiskAccessStorage()
 {
-    stopListsWritingThread();
-
     try
     {
-        writeLists();
+        shutdown();
     }
     catch (...)
     {
@@ -207,6 +205,17 @@ DiskAccessStorage::~DiskAccessStorage()
 }
 
 
+void DiskAccessStorage::shutdown()
+{
+    stopListsWritingThread();
+
+    {
+        std::lock_guard lock{mutex};
+        writeLists();
+    }
+}
+
+
 String DiskAccessStorage::getStorageParamsJSON() const
 {
     std::lock_guard lock{mutex};

src/Access/DiskAccessStorage.h

@@ -18,6 +18,8 @@ public:
     DiskAccessStorage(const String & storage_name_, const String & directory_path_, AccessChangesNotifier & changes_notifier_, bool readonly_, bool allow_backup_);
     ~DiskAccessStorage() override;
 
+    void shutdown() override;
+
     const char * getStorageType() const override { return STORAGE_TYPE; }
     String getStorageParamsJSON() const override;
 

src/Access/IAccessStorage.h

@@ -44,6 +44,11 @@ public:
     explicit IAccessStorage(const String & storage_name_) : storage_name(storage_name_) {}
     virtual ~IAccessStorage() = default;
 
+    /// If the AccessStorage has to do some complicated work when destroying - do it in advance.
+    /// For example, if the AccessStorage contains any threads for background work - ask them to complete and wait for completion.
+    /// By default, does nothing.
+    virtual void shutdown() {}
+
     /// Returns the name of this storage.
     const String & getStorageName() const { return storage_name; }
     virtual const char * getStorageType() const = 0;

src/Access/MultipleAccessStorage.cpp

@@ -34,11 +34,23 @@ MultipleAccessStorage::MultipleAccessStorage(const String & storage_name_)
 
 MultipleAccessStorage::~MultipleAccessStorage()
 {
-    /// It's better to remove the storages in the reverse order because they could depend on each other somehow.
+    try
+    {
+        shutdown();
+    }
+    catch (...)
+    {
+        tryLogCurrentException(__PRETTY_FUNCTION__);
+    }
+}
+
+void MultipleAccessStorage::shutdown()
+{
+    /// It's better to shutdown the storages in the reverse order because they could depend on each other somehow.
     const auto storages = getStoragesPtr();
     for (const auto & storage : *storages | boost::adaptors::reversed)
     {
-        removeStorage(storage);
+        storage->shutdown();
     }
 }
 
@@ -72,6 +84,16 @@ void MultipleAccessStorage::removeStorage(const StoragePtr & storage_to_remove)
     ids_cache.clear();
 }
 
+void MultipleAccessStorage::removeAllStorages()
+{
+    /// It's better to remove the storages in the reverse order because they could depend on each other somehow.
+    const auto storages = getStoragesPtr();
+    for (const auto & storage : *storages | boost::adaptors::reversed)
+    {
+        removeStorage(storage);
+    }
+}
+
 std::vector<StoragePtr> MultipleAccessStorage::getStorages()
 {
     return *getStoragesPtr();

src/Access/MultipleAccessStorage.h

@@ -21,6 +21,8 @@ public:
     explicit MultipleAccessStorage(const String & storage_name_ = STORAGE_TYPE);
     ~MultipleAccessStorage() override;
 
+    void shutdown() override;
+
     const char * getStorageType() const override { return STORAGE_TYPE; }
     bool isReadOnly() const override;
     bool isReadOnly(const UUID & id) const override;
@@ -32,6 +34,7 @@ public:
     void setStorages(const std::vector<StoragePtr> & storages);
     void addStorage(const StoragePtr & new_storage);
     void removeStorage(const StoragePtr & storage_to_remove);
+    void removeAllStorages();
     std::vector<StoragePtr> getStorages();
     std::vector<ConstStoragePtr> getStorages() const;
     std::shared_ptr<const std::vector<StoragePtr>> getStoragesPtr();

src/Access/ReplicatedAccessStorage.cpp

@@ -66,6 +66,18 @@ ReplicatedAccessStorage::ReplicatedAccessStorage(
 }
 
 ReplicatedAccessStorage::~ReplicatedAccessStorage()
+{
+    try
+    {
+        shutdown();
+    }
+    catch (...)
+    {
+        tryLogCurrentException(__PRETTY_FUNCTION__);
+    }
+}
+
+void ReplicatedAccessStorage::shutdown()
 {
     stopWatchingThread();
 }

src/Access/ReplicatedAccessStorage.h

@@ -23,6 +23,8 @@ public:
     ReplicatedAccessStorage(const String & storage_name, const String & zookeeper_path, zkutil::GetZooKeeper get_zookeeper, AccessChangesNotifier & changes_notifier_, bool allow_backup);
     ~ReplicatedAccessStorage() override;
 
+    void shutdown() override;
+
     const char * getStorageType() const override { return STORAGE_TYPE; }
 
     void startPeriodicReloading() override { startWatchingThread(); }

src/Interpreters/Context.cpp

@@ -674,6 +674,9 @@ struct ContextSharedPart : boost::noncopyable
             }
         }
 
+        LOG_TRACE(log, "Shutting down AccessControl");
+        access_control->shutdown();
+
         {
             std::lock_guard lock(mutex);