Fix possible verify crash in case of closing session during keep-alive task and non empry subscrivers queue

If we have subscribers for session and detect error during keep-alive task it is unsafe to return new session to subscriber

4 files changed, 19 insertions, 4 deletions

diff --git a/ydb/public/sdk/cpp/client/resources/ydb_sdk_version.txt b/ydb/public/sdk/cpp/client/resources/ydb_sdk_version.txt
index fad066f801..4fd0fe3cd5 100644
--- a/ydb/public/sdk/cpp/client/resources/ydb_sdk_version.txt
+++ b/ydb/public/sdk/cpp/client/resources/ydb_sdk_version.txt
@@ -1 +1 @@
-2.5.0
\ No newline at end of file
+2.5.1
\ No newline at end of file
diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/session_pool.cpp b/ydb/public/sdk/cpp/client/ydb_table/impl/session_pool.cpp
index 1e360b78af..cdd314c787 100644
--- a/ydb/public/sdk/cpp/client/ydb_table/impl/session_pool.cpp
+++ b/ydb/public/sdk/cpp/client/ydb_table/impl/session_pool.cpp
@@ -160,7 +160,7 @@ TAsyncCreateSessionResult TSessionPool::GetSession(
     return createSessionPromise.GetFuture();
 }
 
-bool TSessionPool::CheckAndFeedWaiterNewSession(std::shared_ptr<TTableClient::TImpl> client) {
+bool TSessionPool::CheckAndFeedWaiterNewSession(std::shared_ptr<TTableClient::TImpl> client, bool active) {
     NThreading::TPromise<TCreateSessionResult> createSessionPromise;
     {
         std::lock_guard guard(Mtx_);
@@ -174,6 +174,21 @@ bool TSessionPool::CheckAndFeedWaiterNewSession(std::shared_ptr<TTableClient::TI
         }
     }
 
+    if (!active) {
+        // Session was IDLE. It means session has been closed during
+        // keep-alive activity inside session pool. In this case
+        // we must not touch ActiveSession counter.
+        // Moreover it is unsafe to recreate the session because session
+        // may be closed during balancing or draining routine.
+        // But we must feed waiters if we have some otherwise deadlock
+        // is possible.
+        // The above mentioned conditions is quite rare so
+        // we can just return an error. In this case uplevel code should
+        // start retry.
+        CreateFakeSession(createSessionPromise, client);
+        return true;
+    }
+
     // This code can be called from client dtors. It may be unsafe to
     // execute grpc call directly...
     client->ScheduleTaskUnsafe([createSessionPromise, client]() mutable {
diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/session_pool.h b/ydb/public/sdk/cpp/client/ydb_table/impl/session_pool.h
index 983d675bce..7a83b4115b 100644
--- a/ydb/public/sdk/cpp/client/ydb_table/impl/session_pool.h
+++ b/ydb/public/sdk/cpp/client/ydb_table/impl/session_pool.h
@@ -51,7 +51,7 @@ public:
     bool ReturnSession(TSession::TImpl* impl, bool active, std::shared_ptr<TTableClient::TImpl> client);
     // Returns trun if has waiter and scheduled to create new session
     // too feed it
-    bool CheckAndFeedWaiterNewSession(std::shared_ptr<TTableClient::TImpl> client);
+    bool CheckAndFeedWaiterNewSession(std::shared_ptr<TTableClient::TImpl> client, bool active);
     TPeriodicCb CreatePeriodicTask(std::weak_ptr<TTableClient::TImpl> weakClient, TKeepAliveCmd&& cmd, TDeletePredicate&& predicate);
     i64 GetActiveSessions() const;
     i64 GetActiveSessionsLimit() const;
diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp
index 096153cbcf..56974b411a 100644
--- a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp
+++ b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp
@@ -867,7 +867,7 @@ bool TTableClient::TImpl::ReturnSession(TSession::TImpl* sessionImpl) {
 void TTableClient::TImpl::DeleteSession(TSession::TImpl* sessionImpl) {
     // Closing not owned by session pool session should not fire getting new session
     if (sessionImpl->IsOwnedBySessionPool()) {
-        if (SessionPool_.CheckAndFeedWaiterNewSession(shared_from_this())) {
+        if (SessionPool_.CheckAndFeedWaiterNewSession(shared_from_this(), sessionImpl->NeedUpdateActiveCounter())) {
             // We requested new session for waiter which already incremented
             // active session counter and old session will be deleted
             // - skip update active counter in this case