From 9a8fe206a46ec2a2a710a987cc82404fcf07abb4 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Fri, 14 Aug 2026 14:07:49 +0200 Subject: [PATCH] Let a session serve several callers without one closing it One session is now routinely handed to a console and a command at once, and logout closed it for both, leaving whoever was left holding one that answered as though it had been lost. Count the holders and give up a claim instead, unless the session is no longer usable, which logout is told by sessionok. A console had no way to give a claim back: close deactivated its sol payload and left the session alone, which was right when closing meant closing it for everybody and is a leak now. Both of its exits release it. --- confluent_server/aiohmi/ipmi/console.py | 22 +++++++++++++++---- .../aiohmi/ipmi/private/session.py | 21 +++++++++++++++--- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/confluent_server/aiohmi/ipmi/console.py b/confluent_server/aiohmi/ipmi/console.py index c68ca774..6f3abfeb 100644 --- a/confluent_server/aiohmi/ipmi/console.py +++ b/confluent_server/aiohmi/ipmi/console.py @@ -204,6 +204,21 @@ class Console(object): if not self.awaitingack: await self._sendpendingoutput() + async def _release_session(self): + """Give up this console's claim on the ipmi session + + A console can be sharing one with whatever else is talking to the same + bmc, so let go of it rather than closing it. Clearing the reference + first means it does not matter how many exits end up here. + """ + sess = self.ipmi_session + if sess is None: + return + self.ipmi_session = None + if sess.sol_handler is not None and sess.sol_handler.__self__ is self: + sess.sol_handler = None + await sess.logout() + async def close(self): """Shut down an SOL session""" @@ -217,6 +232,7 @@ class Console(object): # if underlying ipmi session is not working, then # run with the implicit success pass + await self._release_session() async def send_data(self, data): if self.broken: @@ -320,10 +336,8 @@ class Console(object): self.broken = True if self.ipmi_session: self.ipmi_session.unregister_keepalive(self.keepaliveid) - if (self.ipmi_session.sol_handler - and self.ipmi_session.sol_handler.__self__ is self): - self.ipmi_session.sol_handler = None - self.ipmi_session = None + # A console that has given up is finished with the session too + await self._release_session() if type(error) == dict: await self._print_data(error) else: diff --git a/confluent_server/aiohmi/ipmi/private/session.py b/confluent_server/aiohmi/ipmi/private/session.py index 5ce8e9e7..a13d9cb3 100644 --- a/confluent_server/aiohmi/ipmi/private/session.py +++ b/confluent_server/aiohmi/ipmi/private/session.py @@ -479,10 +479,15 @@ class Session(object): # id, however it's easier this way forbidsock.append(self.socket) if trueself: - return await cls._await_login(trueself) + await cls._await_login(trueself) + # Count the caller only once it is really getting the session + trueself.users += 1 + return trueself i = cls.initting_sessions.get(sesskey, False) if i: - return await cls._await_login(i) + await cls._await_login(i) + i.users += 1 + return i self = super().__new__(cls) self.forbidsock = forbidsock # Register before establishing, not after: this is where a caller @@ -531,6 +536,8 @@ class Session(object): # Whether this session still holds the socket pool count it took. # Set before anything can fail, so releasing is safe either way. self._socketclaimed = False + # How many callers hold this session; the last one out closes it + self.users = 1 self.lastpayload = None self._customkeepalives = None # queue of events denoting line to run a cmd @@ -1936,7 +1943,15 @@ class Session(object): WAITING_SESSIONS.release() async def logout(self, sessionok=True): - + if (sessionok and self.users > 1 + and not self.broken and not self.cleaningup): + # A caller finished with a working session gives up its claim and + # leaves it to whoever else holds it; closing it here would hand + # them one that answers as though it had been lost. sessionok is + # false when it is no longer usable, and then it comes down anyway. + self.users -= 1 + return {'success': True} + self.users = 0 if self.cleaningup: self.nowait = True if self.logged: