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
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 @@
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 @@
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 @@
/* Associate session with handler */
sessions[*session] = SessionInfo(path, handler, flags);
+ openSessions[handler].insert(*session);
incrementOpen(path);
return true;
}
@@ -206,8 +258,7 @@
{
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 btTransportLength = 64;
const uint32_t maximumReadSize =
btTransportLength - (btReplyHdrLen + oem::groupMagicSize + crcSize);
+constexpr auto defaultSessionTimeout = 10min;
struct SessionInfo
{
@@ -85,7 +89,8 @@
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 @@
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 @@
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;
};
/**