From b61a88bacdd5fdd2507cb218afcf95c40a2565f2 Mon Sep 17 00:00:00 2001
From: Kun Yi <kunyi731@gmail.com>
Date: Tue, 12 Nov 2019 22:50:34 -0800
Subject: Implement session expiration

If the caller opens sessions but doesn't close them, either
intentionally or unintentionally, session IDs will leak. Once the
maximum number of sessions are reached, no new session can be opened.

Implement a cleanup procedure to automatically remove stale sessions. If
a session hasn't seen activity for a while, call the expire()
functions on the handler, and remove the session from tracking table.
For handlers that haven't implemented the expire() call this change will
be a no-op.

Signed-off-by: Kun Yi <kunyi731@gmail.com>
Change-Id: I895ae19b4003d2d6f7a0b2e73370fe5aa664adee
---
 manager.cpp | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 manager.hpp | 16 +++++++++++++++-
 2 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/manager.cpp b/manager.cpp
index 684f20c..286b81a 100644
--- a/manager.cpp
+++ b/manager.cpp
@@ -17,6 +17,7 @@
 #include "manager.hpp"
 
 #include <algorithm>
+#include <iostream>
 #include <memory>
 #include <string>
 #include <vector>
@@ -67,6 +68,52 @@ int BlobManager::getOpen(const std::string& path) const
     return 0;
 }
 
+void BlobManager::eraseSession(GenericBlobInterface* handler, uint16_t session)
+{
+    sessions.erase(session);
+    /* Ok for openSessions[handler] to be an empty set */
+    openSessions[handler].erase(session);
+    decrementOpen(getPath(session));
+}
+
+void BlobManager::cleanUpStaleSessions(GenericBlobInterface* handler)
+{
+    if (openSessions.count(handler) == 0)
+    {
+        return;
+    }
+
+    auto timeNow = std::chrono::steady_clock::now();
+    std::set<uint16_t> expiredSet;
+
+    for (auto sessionId : openSessions[handler])
+    {
+        if (timeNow - sessions[sessionId].lastActionTime >= sessionTimeout)
+        {
+            expiredSet.insert(sessionId);
+        }
+    }
+
+    for (auto sessionId : expiredSet)
+    {
+        std::cerr << "phosphor-ipmi-blobs: expiring stale session " << sessionId
+                  << std::endl;
+
+        /* We do a best case recovery by issuing an expire call. If it fails
+         * don't erase sessions since the handler side might be still tracking
+         * it as open. */
+        if (handler->expire(sessionId))
+        {
+            eraseSession(handler, sessionId);
+        }
+        else
+        {
+            std::cerr << "phosphor-ipmi-blobs: failed to expire session "
+                      << sessionId << std::endl;
+        }
+    }
+}
+
 bool BlobManager::registerHandler(std::unique_ptr<GenericBlobInterface> handler)
 {
     if (!handler)
@@ -130,6 +177,10 @@ bool BlobManager::open(uint16_t flags, const std::string& path,
         return false;
     }
 
+    /* Try to clean up anything that's falling out of cleanup timeout for this
+     * handler */
+    cleanUpStaleSessions(handler);
+
     if (!handler->open(*session, flags, path))
     {
         return false;
@@ -137,6 +188,7 @@ bool BlobManager::open(uint16_t flags, const std::string& path,
 
     /* Associate session with handler */
     sessions[*session] = SessionInfo(path, handler, flags);
+    openSessions[handler].insert(*session);
     incrementOpen(path);
     return true;
 }
@@ -206,8 +258,7 @@ bool BlobManager::close(uint16_t session)
         {
             return false;
         }
-        sessions.erase(session);
-        decrementOpen(getPath(session));
+        eraseSession(handler, session);
         return true;
     }
     return false;
diff --git a/manager.hpp b/manager.hpp
index e7a9a97..011fe88 100644
--- a/manager.hpp
+++ b/manager.hpp
@@ -5,6 +5,7 @@
 #include <ctime>
 #include <ipmid/oemrouter.hpp>
 #include <memory>
+#include <set>
 #include <string>
 #include <unordered_map>
 #include <vector>
@@ -12,6 +13,8 @@
 namespace blobs
 {
 
+using namespace std::chrono_literals;
+
 /* The maximum read size.
  * NOTE: Once this can be dynamically determined, we'll switch to that method.
  * Having this in a header allows it to used cleanly for now.
@@ -21,6 +24,7 @@ const int btReplyHdrLen = 5;
 const int btTransportLength = 64;
 const uint32_t maximumReadSize =
     btTransportLength - (btReplyHdrLen + oem::groupMagicSize + crcSize);
+constexpr auto defaultSessionTimeout = 10min;
 
 struct SessionInfo
 {
@@ -85,7 +89,8 @@ class ManagerInterface
 class BlobManager : public ManagerInterface
 {
   public:
-    BlobManager()
+    BlobManager(std::chrono::seconds sessionTimeout = defaultSessionTimeout) :
+        sessionTimeout(sessionTimeout)
     {
         next = static_cast<uint16_t>(std::time(nullptr));
     };
@@ -275,6 +280,13 @@ class BlobManager : public ManagerInterface
     void decrementOpen(const std::string& path);
     int getOpen(const std::string& path) const;
 
+    /* Helper method to erase a session from all maps */
+    void eraseSession(GenericBlobInterface* handler, uint16_t session);
+    /* For each session owned by this handler, call expire if it is stale */
+    void cleanUpStaleSessions(GenericBlobInterface* handler);
+
+    /* How long a session has to be inactive to be considered stale */
+    std::chrono::seconds sessionTimeout;
     /* The next session ID to use */
     uint16_t next;
     /* Temporary list of blobIds used for enumeration. */
@@ -286,6 +298,8 @@ class BlobManager : public ManagerInterface
     std::unordered_map<uint16_t, SessionInfo> sessions;
     /* Mapping of open blobIds */
     std::unordered_map<std::string, int> openFiles;
+    /* Map of handlers to their open sessions */
+    std::unordered_map<GenericBlobInterface*, std::set<uint16_t>> openSessions;
 };
 
 /**
-- 
cgit v1.2.3