destroy stale sessions without a lock

tavplubix · tavplubix · commit 4556e746dcc1 · 2025-06-04T19:39:25.000+02:00
diff --git a/src/Interpreters/Session.cpp b/src/Interpreters/Session.cpp
@@ -226,14 +226,18 @@ class NamedSessionsStorage
         std::unique_lock lock{mutex};
         while (!quit)
         {
-            closeSessions(lock);
+            auto closed_sessions = closeSessions(lock);
+            lock.unlock();
+            closed_sessions.clear();
+            lock.lock();
             if (cond.wait_for(lock, close_interval, [this]() -> bool { return quit; }))
                 break;
         }
     }
 
-    void closeSessions(std::unique_lock<std::mutex> & lock)
+    std::vector<std::shared_ptr<NamedSessionData>> closeSessions(std::unique_lock<std::mutex> & lock)
     {
+        std::vector<std::shared_ptr<NamedSessionData>> closed_sessions;
         const auto now = std::chrono::steady_clock::now();
 
         for (auto bucket_it = close_time_buckets.begin(); bucket_it != close_time_buckets.end(); bucket_it = close_time_buckets.erase(bucket_it))
@@ -270,9 +274,12 @@ class NamedSessionsStorage
                 }
 
                 LOG_TRACE(log, "Close session with session_id: {}, user_id: {}", key.second, toString(key.first));
+                closed_sessions.push_back(session);
                 sessions.erase(session_it);
             }
         }
+
+        return closed_sessions;
     }
 
     std::mutex mutex;

Original file line number	Diff line number	Diff line change
`@@ -226,14 +226,18 @@ class NamedSessionsStorage`
`226`	`226`	`std::unique_lock lock{mutex};`
`227`	`227`	`while (!quit)`
`228`	`228`	`{`
`229`		`- closeSessions(lock);`
	`229`	`+ auto closed_sessions = closeSessions(lock);`
	`230`	`+ lock.unlock();`
	`231`	`+ closed_sessions.clear();`
	`232`	`+ lock.lock();`
`230`	`233`	`if (cond.wait_for(lock, close_interval, [this]() -> bool { return quit; }))`
`231`	`234`	`break;`
`232`	`235`	`}`
`233`	`236`	`}`
`234`	`237`
`235`		`- void closeSessions(std::unique_lock<std::mutex> & lock)`
	`238`	`+ std::vector<std::shared_ptr<NamedSessionData>> closeSessions(std::unique_lock<std::mutex> & lock)`
`236`	`239`	`{`
	`240`	`+ std::vector<std::shared_ptr<NamedSessionData>> closed_sessions;`
`237`	`241`	`const auto now = std::chrono::steady_clock::now();`
`238`	`242`
`239`	`243`	`for (auto bucket_it = close_time_buckets.begin(); bucket_it != close_time_buckets.end(); bucket_it = close_time_buckets.erase(bucket_it))`
`@@ -270,9 +274,12 @@ class NamedSessionsStorage`
`270`	`274`	`}`
`271`	`275`
`272`	`276`	`LOG_TRACE(log, "Close session with session_id: {}, user_id: {}", key.second, toString(key.first));`
	`277`	`+ closed_sessions.push_back(session);`
`273`	`278`	`sessions.erase(session_it);`
`274`	`279`	`}`
`275`	`280`	`}`
	`281`	`+`
	`282`	`+ return closed_sessions;`
`276`	`283`	`}`
`277`	`284`
`278`	`285`	`std::mutex mutex;`