Improve Session Management for Charge Point Connections

[Sn0w3y]

Summary

This PR addresses the issue of improper session management when multiple connections are established to the same charge point. Specifically, it ensures that only one active session per charge point is maintained, and existing sessions are properly terminated before establishing new ones. This update helps prevent unintended side effects where disconnecting an older connection impacts newer connections.

Changes Made

1. Handle Existing Sessions When a New Connection is Established

   • Updated newSession method in MyJsonServer.java:
     • Checks if an existing session already exists for a given charge point.
     • If found, gracefully terminates the old session by invoking lostSession() and removes it from active sessions.
     • Associates the new session with the charge point and initializes it accordingly.

2. Prevent lostSession from Affecting New Connections

   • Updated lostSession method in MyJsonServer.java:
     • Ensures that only the intended session is terminated without affecting other active connections.
     • Before removing a session from ocppSessions, it verifies that the session being disconnected is still the active one.

3. Ensure Proper Cleanup in removeEvcs

   • Reviewed and modified removeEvcs method in EvcsOcppServer.java:
     • Ensures that removing an EVCS component correctly handles session mappings without affecting other active sessions.
     • Properly removes the EVCS component from session lists and invokes lostSession() for cleanup.

Additional Enhancements

• Thread Safety: Made sure that shared resources such as activeEvcsSessions and ocppSessions are accessed in a thread-safe manner.
• Logging Improvements: Enhanced logging to include more contextual information to facilitate easier debugging and monitoring of session activities.
• Session Validation: Added additional checks to validate the integrity of sessions to ensure correct mapping between charge points and sessions.
• Resource Cleanup: Ensured proper resource cleanup upon session termination to prevent memory leaks and stale references.

Testing

• Please test this Code Improvement as i do not have the ability to test it on my side.
• @katsuya please take this task maybe as you have the issue right now in:
  OCPP server fail to send commands to the charging stations when two connections are established #2906

Motivation

These changes address the observed problem where OpenEMS failed to handle multiple simultaneous connections to the same charge point, causing instability in session management. By implementing these changes, the system will now correctly manage and isolate sessions, ensuring reliable operation of charge point connections.

Related Issues

• Fixes improper handling of multiple charge point connections.
• Resolves issues with old connections impacting newly established sessions.

Notes

If further refinements are needed, or any issues persist with session handling, please feel free to provide feedback.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Additional details and impacted files

@@              Coverage Diff              @@
##             develop    #2908      +/-   ##
=============================================
+ Coverage      56.63%   56.64%   +0.01%     
- Complexity      9513     9515       +2     
=============================================
  Files           2253     2253              
  Lines          96074    96074              
  Branches        7095     7095              
=============================================
+ Hits           54401    54408       +7     
+ Misses         39666    39658       -8     
- Partials        2007     2008       +1     

[katsuya]

@Sn0w3y Thanks a lot! We'll give the provided patch a try.

Thread Safety: Made sure that shared resources such as activeEvcsSessions and ocppSessions are accessed in a thread-safe manner.

This is absolutely true. Occasionally, we also experience erratic behavior, and it's sometimes difficult to determine if this is the cause, which can be quite frustrating.

By the way, here’s the fix we implemented. It mostly resolved the issue, but we’re still experiencing some very rare instances of erratic behavior, which is troubling.

From 342832e40892ae147dc020edd37d93fc297b52e1 Mon Sep 17 00:00:00 2001
From: Katsuya Oda <[email protected]>
Date: Wed, 4 Dec 2024 09:05:59 +0900
Subject: [PATCH] fix: remove previous session

---
 .../src/io/openems/edge/evcs/ocpp/server/MyJsonServer.java  | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/io.openems.edge.evcs.ocpp.server/src/io/openems/edge/evcs/ocpp/server/MyJsonServer.java b/io.openems.edge.evcs.ocpp.server/src/io/openems/edge/evcs/ocpp/server/MyJsonServer.java
index 66d4d78b3..e461755a2 100644
--- a/io.openems.edge.evcs.ocpp.server/src/io/openems/edge/evcs/ocpp/server/MyJsonServer.java
+++ b/io.openems.edge.evcs.ocpp.server/src/io/openems/edge/evcs/ocpp/server/MyJsonServer.java
@@ -90,7 +90,7 @@ public class MyJsonServer {
 
 				var ocppIdentifier = information.getIdentifier().replace("/", "");
 
-				MyJsonServer.this.parent.ocppSessions.put(ocppIdentifier, sessionIndex);
+				var sessionIndexPrevious = MyJsonServer.this.parent.ocppSessions.put(ocppIdentifier, sessionIndex);
 
 				var presentEvcss = MyJsonServer.this.parent.ocppEvcss.get(ocppIdentifier);
 
@@ -103,6 +103,10 @@ public class MyJsonServer {
 					evcs.newSession(MyJsonServer.this.parent, sessionIndex);
 					MyJsonServer.this.sendInitialRequests(sessionIndex, evcs);
 				}
+
+				if (sessionIndexPrevious != null) {
+					MyJsonServer.this.parent.activeEvcsSessions.remove(sessionIndexPrevious);
+				}
 			}
 
 			@Override
-- 
2.39.5 (Apple Git-154)

[Sn0w3y]

By the way, here’s the fix we implemented. It mostly resolved the issue, but we’re still experiencing some very rare instances of erratic behavior, which is troubling.

@katsuya Thank you for your efforts in addressing the session management issue aswell!
However, my fix provides a more comprehensive solution by not only removing the previous session but also invoking lostSession() to ensure complete cleanup.
Additionally, it includes verification to confirm that only the intended session is terminated, enhancing thread safety and preventing any unintended impact on new connections.

Greetings

[katsuya]

@Sn0w3y It seems that the fix for the following issue has been removed and replaced with the changes from #2912. Was this intentional? Since the root causes are different, I think it would be better to address them separately.

When a new connection is established, the old connection must be correctly removed from activeEvcSessions.

[Sn0w3y]

@katsuya you are invited to do the WIP please :)