Skip to content

Commit 2dc6c7a

Browse files
committed
feat(stream): bridge SessionManager termination to SSE transport cleanup
Sessions can terminate through paths that bypass the controller — user session-limit eviction, grace-period expiry, idle cleanup — and the previous wiring left transport-level state (active emitter, broadcaster sender, heartbeat task) leaking on those paths. Inject the SseStreamController via ObjectProvider into StreamComponentWiring and invoke releaseTransportResources(sessionId) inside the onSessionTerminated callback so every termination path releases the same resources. The call is idempotent and safe to re-run when the controller already released them via cleanupSession(). SseStreamController.cleanupSession is split: transport release moves to the public releaseTransportResources(), and the controller-driven session cleanup orchestrates subscription cleanup, transport release, and SessionManager.terminateSession() in that order. setupCleanupCallbacks gains an AtomicBoolean CAS gate because Spring fires both onTimeout and onCompletion when an emitter times out, and without dedup the cleanup work — including the log line and listener wake-ups — duplicated. SessionManager.completeTermination now runs the onSessionTerminated callback BEFORE clearing the session's subscriptions, so listeners can iterate session.getSubscriptions() to release per-key state. The callback is wrapped in try/catch so a misbehaving listener cannot leave the session half-cleaned. Registry unregistration moves ahead of the callback so concurrent lookups observe the termination immediately. The user-limit branch in registerSession now evicts the oldest session (FIFO) explicitly to admit the new connection, matching the behaviour that was already implemented but only a comment-deep.
1 parent 55f0462 commit 2dc6c7a

6 files changed

Lines changed: 211 additions & 29 deletions

File tree

simplix-stream/src/main/java/dev/simplecore/simplix/stream/autoconfigure/SimpliXStreamCoreConfiguration.java

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414
import dev.simplecore.simplix.stream.infrastructure.local.LocalBroadcaster;
1515
import dev.simplecore.simplix.stream.infrastructure.local.LocalSessionRegistry;
1616
import dev.simplecore.simplix.stream.persistence.service.DbSessionRegistry;
17+
import dev.simplecore.simplix.stream.transport.sse.SseStreamController;
1718
import lombok.extern.slf4j.Slf4j;
19+
import org.springframework.beans.factory.ObjectProvider;
1820
import org.springframework.beans.factory.annotation.Autowired;
1921
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
2022
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
@@ -161,18 +163,24 @@ public StreamComponentWiring streamComponentWiring(
161163
SessionManager sessionManager,
162164
StreamProperties properties,
163165
SimpliXStreamDataCollectorRegistry collectorRegistry,
166+
ObjectProvider<SseStreamController> sseControllerProvider,
164167
@Autowired(required = false) EventStreamHandler eventStreamHandler,
165168
@Autowired(required = false) SimpliXStreamEventSourceRegistry eventSourceRegistry) {
166169
return new StreamComponentWiring(
167170
subscriptionManager, schedulerManager, sessionManager, properties,
168-
collectorRegistry, eventStreamHandler, eventSourceRegistry);
171+
collectorRegistry, sseControllerProvider.getIfAvailable(),
172+
eventStreamHandler, eventSourceRegistry);
169173
}
170174

171175
/**
172176
* Wires stream components together after construction.
173177
* <p>
174178
* Routes subscriptions to either SchedulerManager (polling) or EventStreamHandler (event-based)
175179
* depending on whether a StreamEventSource is registered for the resource.
180+
* Also bridges {@link SessionManager} termination events back to the SSE transport
181+
* so non-controller-driven termination paths (user-limit eviction, grace expiry,
182+
* idle cleanup) still release transport-level resources (active emitter, broadcaster
183+
* sender, heartbeat task).
176184
*/
177185
@Slf4j
178186
public static class StreamComponentWiring {
@@ -183,6 +191,7 @@ public StreamComponentWiring(
183191
SessionManager sessionManager,
184192
StreamProperties properties,
185193
SimpliXStreamDataCollectorRegistry collectorRegistry,
194+
SseStreamController sseStreamController,
186195
EventStreamHandler eventStreamHandler,
187196
SimpliXStreamEventSourceRegistry eventSourceRegistry) {
188197

@@ -228,7 +237,9 @@ public StreamComponentWiring(
228237
}
229238
});
230239

231-
// Wire session termination to subscription cleanup
240+
// Wire session termination to subscription cleanup AND transport resource release.
241+
// SessionManager invokes this callback BEFORE clearing the session's subscriptions,
242+
// so iterating session.getSubscriptions() yields the live keys.
232243
sessionManager.setOnSessionTerminated(session -> {
233244
for (var key : session.getSubscriptions()) {
234245
String resource = key.getResource();
@@ -244,6 +255,13 @@ public StreamComponentWiring(
244255
if (eventSourceEnabled) {
245256
eventStreamHandler.removeSubscriberFromAll(session.getId());
246257
}
258+
259+
// Release SSE transport resources for paths that bypass the controller
260+
// (user-limit eviction, grace expiry, idle cleanup). Idempotent — safe to
261+
// re-run when the controller already released them in cleanupSession().
262+
if (sseStreamController != null) {
263+
sseStreamController.releaseTransportResources(session.getId());
264+
}
247265
});
248266

249267
if (eventSourceEnabled) {

simplix-stream/src/main/java/dev/simplecore/simplix/stream/core/session/SessionManager.java

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,9 @@ public StreamSession createSession(String userId, TransportType transportType) {
8484
long currentCount = sessionRegistry.countByUserId(userId);
8585
if (currentCount >= maxPerUser) {
8686
log.warn("User {} has reached session limit: {}/{}", userId, currentCount, maxPerUser);
87-
// Option: terminate oldest session or reject new connection
88-
// Currently rejecting - can be configured
87+
// Policy: evict oldest session (FIFO) to admit the new connection.
88+
// Alternative would be to reject the new connection — make this configurable
89+
// via properties when both policies are needed.
8990
terminateOldestSession(userId);
9091
}
9192
}
@@ -295,25 +296,32 @@ public Set<SubscriptionKey> terminateSession(String sessionId) {
295296
* Complete session termination after state is marked as terminated.
296297
* <p>
297298
* This method should be called after acquiring gracePeriodLock and marking
298-
* the session as terminated.
299+
* the session as terminated. The termination callback runs while the
300+
* session's subscriptions are still populated so listeners can iterate
301+
* them; the subscriptions are cleared afterwards.
299302
*
300303
* @param session the session to terminate
301304
* @return the cleared subscriptions
302305
*/
303306
private Set<SubscriptionKey> completeTermination(StreamSession session) {
304-
Set<SubscriptionKey> clearedSubscriptions = session.clearSubscriptions();
305-
306-
// Unregister from registry
307+
// Unregister from registry first so concurrent lookups see termination
307308
sessionRegistry.unregister(session.getId());
308309

309-
log.info("Session terminated: {} (user={}, subscriptions={})",
310-
session.getId(), session.getUserId(), clearedSubscriptions.size());
311-
312-
// Notify callback
310+
// Notify callback BEFORE clearing subscriptions so listeners can read them
313311
if (onSessionTerminated != null) {
314-
onSessionTerminated.accept(session);
312+
try {
313+
onSessionTerminated.accept(session);
314+
} catch (RuntimeException e) {
315+
log.warn("onSessionTerminated callback threw for session {}: {}",
316+
session.getId(), e.getMessage(), e);
317+
}
315318
}
316319

320+
Set<SubscriptionKey> clearedSubscriptions = session.clearSubscriptions();
321+
322+
log.info("Session terminated: {} (user={}, subscriptions={})",
323+
session.getId(), session.getUserId(), clearedSubscriptions.size());
324+
317325
return clearedSubscriptions;
318326
}
319327

simplix-stream/src/main/java/dev/simplecore/simplix/stream/transport/sse/SseStreamController.java

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import java.time.Duration;
3333
import java.util.*;
3434
import java.util.concurrent.*;
35+
import java.util.concurrent.atomic.AtomicBoolean;
3536

3637
/**
3738
* REST controller for SSE streaming connections.
@@ -325,7 +326,16 @@ private void sendReconnectionMessage(SseStreamSession sseSession, StreamSession
325326
}
326327

327328
private void setupCleanupCallbacks(SseEmitter emitter, String sessionId) {
329+
// Spring fires both onTimeout AND onCompletion when an SSE emitter times out
330+
// (timeout callback first, then completion when the response is finalized).
331+
// Without dedup the cleanup runs twice — markDisconnected is internally guarded
332+
// but the log line and listener wake-ups still duplicate. CAS gate runs cleanup
333+
// exactly once per emitter regardless of which callback fires first.
334+
AtomicBoolean cleaned = new AtomicBoolean(false);
328335
Runnable cleanup = () -> {
336+
if (!cleaned.compareAndSet(false, true)) {
337+
return;
338+
}
329339
log.debug("SSE connection closed, starting grace period for session: {}", sessionId);
330340
SseStreamSession sseSession = activeSessions.get(sessionId);
331341
if (sseSession != null) {
@@ -394,22 +404,44 @@ private void sendConnectionMessage(SseStreamSession sseSession, String sessionId
394404
}
395405

396406
private void cleanupSession(String sessionId) {
397-
SseStreamSession sseSession = activeSessions.remove(sessionId);
398-
if (sseSession == null) {
407+
if (!activeSessions.containsKey(sessionId)) {
399408
return; // Already cleaned up — idempotent guard
400409
}
401410

402-
cancelHeartbeat(sessionId);
403-
sseSession.close();
404-
405-
broadcaster.unregisterSender(sessionId);
411+
// Release transport resources first; per-key subscription cleanup runs through
412+
// SubscriptionManager so scheduler/event listeners receive removal notifications.
406413
subscriptionManager.clearSubscriptions(sessionId);
414+
releaseTransportResources(sessionId);
407415
sessionManager.terminateSession(sessionId);
416+
417+
log.info("Session cleaned up: {}", sessionId);
418+
}
419+
420+
/**
421+
* Release transport-level resources for a session.
422+
* <p>
423+
* Closes the SSE emitter, removes the session from the active map and the
424+
* broadcaster's sender registry, cancels the heartbeat task, and removes the
425+
* session from any event-source registries. All operations are idempotent so
426+
* this method is safe to call multiple times for the same session ID.
427+
* <p>
428+
* Invoked from two paths: (a) controller-driven cleanup (explicit disconnect,
429+
* heartbeat-detected invalid session), and (b) the wiring-config callback when
430+
* SessionManager terminates a session without going through the controller
431+
* (user-limit eviction, grace-period expiry).
432+
*
433+
* @param sessionId the session ID
434+
*/
435+
public void releaseTransportResources(String sessionId) {
436+
cancelHeartbeat(sessionId);
437+
SseStreamSession sseSession = activeSessions.remove(sessionId);
438+
if (sseSession != null) {
439+
sseSession.close();
440+
}
441+
broadcaster.unregisterSender(sessionId);
408442
if (eventStreamHandler != null) {
409443
eventStreamHandler.removeSubscriberFromAll(sessionId);
410444
}
411-
412-
log.info("Session cleaned up: {}", sessionId);
413445
}
414446

415447
private List<Subscription> convertToSubscriptions(SubscriptionRequest request, StreamSession session) {

simplix-stream/src/test/java/dev/simplecore/simplix/stream/autoconfigure/StreamComponentWiringTest.java

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import dev.simplecore.simplix.stream.core.subscription.SubscriptionManager;
1111
import dev.simplecore.simplix.stream.eventsource.EventStreamHandler;
1212
import dev.simplecore.simplix.stream.eventsource.SimpliXStreamEventSourceRegistry;
13+
import dev.simplecore.simplix.stream.transport.sse.SseStreamController;
1314
import org.junit.jupiter.api.BeforeEach;
1415
import org.junit.jupiter.api.DisplayName;
1516
import org.junit.jupiter.api.Nested;
@@ -80,7 +81,7 @@ void shouldRouteToSchedulerManager() {
8081

8182
new SimpliXStreamCoreConfiguration.StreamComponentWiring(
8283
subscriptionManager, schedulerManager, sessionManager, properties,
83-
collectorRegistry, null, null);
84+
collectorRegistry, null, null, null);
8485

8586
verify(subscriptionManager).setOnSubscriptionAdded(addCaptor.capture());
8687
verify(subscriptionManager).setOnSubscriptionRemoved(removeCaptor.capture());
@@ -102,7 +103,7 @@ void shouldUseCollectorInterval() {
102103

103104
new SimpliXStreamCoreConfiguration.StreamComponentWiring(
104105
subscriptionManager, schedulerManager, sessionManager, properties,
105-
collectorRegistry, null, null);
106+
collectorRegistry, null, null, null);
106107

107108
verify(subscriptionManager).setOnSubscriptionAdded(addCaptor.capture());
108109

@@ -125,7 +126,7 @@ void shouldRouteRemovalToScheduler() {
125126

126127
new SimpliXStreamCoreConfiguration.StreamComponentWiring(
127128
subscriptionManager, schedulerManager, sessionManager, properties,
128-
collectorRegistry, null, null);
129+
collectorRegistry, null, null, null);
129130

130131
verify(subscriptionManager).setOnSubscriptionRemoved(removeCaptor.capture());
131132

@@ -143,7 +144,7 @@ void shouldCleanUpOnSessionTermination() {
143144

144145
new SimpliXStreamCoreConfiguration.StreamComponentWiring(
145146
subscriptionManager, schedulerManager, sessionManager, properties,
146-
collectorRegistry, null, null);
147+
collectorRegistry, null, null, null);
147148

148149
verify(sessionManager).setOnSessionTerminated(terminateCaptor.capture());
149150

@@ -173,7 +174,7 @@ void shouldRouteToEventHandler() {
173174

174175
new SimpliXStreamCoreConfiguration.StreamComponentWiring(
175176
subscriptionManager, schedulerManager, sessionManager, properties,
176-
collectorRegistry, eventStreamHandler, eventSourceRegistry);
177+
collectorRegistry, null, eventStreamHandler, eventSourceRegistry);
177178

178179
verify(subscriptionManager).setOnSubscriptionAdded(addCaptor.capture());
179180

@@ -196,7 +197,7 @@ void shouldRouteRemovalToEventHandler() {
196197

197198
new SimpliXStreamCoreConfiguration.StreamComponentWiring(
198199
subscriptionManager, schedulerManager, sessionManager, properties,
199-
collectorRegistry, eventStreamHandler, eventSourceRegistry);
200+
collectorRegistry, null, eventStreamHandler, eventSourceRegistry);
200201

201202
verify(subscriptionManager).setOnSubscriptionRemoved(removeCaptor.capture());
202203

@@ -219,7 +220,7 @@ void shouldCleanUpFromEventHandlerOnTermination() {
219220

220221
new SimpliXStreamCoreConfiguration.StreamComponentWiring(
221222
subscriptionManager, schedulerManager, sessionManager, properties,
222-
collectorRegistry, eventStreamHandler, eventSourceRegistry);
223+
collectorRegistry, null, eventStreamHandler, eventSourceRegistry);
223224

224225
verify(sessionManager).setOnSessionTerminated(terminateCaptor.capture());
225226

@@ -234,6 +235,52 @@ void shouldCleanUpFromEventHandlerOnTermination() {
234235
verify(eventStreamHandler).removeSubscriberFromAll("session-1");
235236
}
236237

238+
@Test
239+
@DisplayName("should release SSE transport resources on session termination when controller present")
240+
void shouldReleaseSseTransportResourcesOnTermination() {
241+
ArgumentCaptor<Consumer<StreamSession>> terminateCaptor =
242+
ArgumentCaptor.forClass(Consumer.class);
243+
SseStreamController sseController = mock(SseStreamController.class);
244+
245+
when(eventSourceRegistry.getRegisteredResources()).thenReturn(Set.of("events"));
246+
247+
new SimpliXStreamCoreConfiguration.StreamComponentWiring(
248+
subscriptionManager, schedulerManager, sessionManager, properties,
249+
collectorRegistry, sseController, eventStreamHandler, eventSourceRegistry);
250+
251+
verify(sessionManager).setOnSessionTerminated(terminateCaptor.capture());
252+
253+
StreamSession session = mock(StreamSession.class);
254+
when(session.getSubscriptions()).thenReturn(Set.of());
255+
when(session.getId()).thenReturn("session-leak-fix");
256+
257+
terminateCaptor.getValue().accept(session);
258+
259+
verify(sseController).releaseTransportResources("session-leak-fix");
260+
}
261+
262+
@Test
263+
@DisplayName("should not fail termination callback when SSE controller is absent")
264+
void shouldHandleAbsentSseControllerOnTermination() {
265+
ArgumentCaptor<Consumer<StreamSession>> terminateCaptor =
266+
ArgumentCaptor.forClass(Consumer.class);
267+
268+
when(eventSourceRegistry.getRegisteredResources()).thenReturn(Set.of("events"));
269+
270+
new SimpliXStreamCoreConfiguration.StreamComponentWiring(
271+
subscriptionManager, schedulerManager, sessionManager, properties,
272+
collectorRegistry, null, eventStreamHandler, eventSourceRegistry);
273+
274+
verify(sessionManager).setOnSessionTerminated(terminateCaptor.capture());
275+
276+
StreamSession session = mock(StreamSession.class);
277+
when(session.getSubscriptions()).thenReturn(Set.of());
278+
when(session.getId()).thenReturn("session-no-sse");
279+
280+
// Must not throw — wiring should tolerate WebSocket-only or no-transport deployments
281+
terminateCaptor.getValue().accept(session);
282+
}
283+
237284
@Test
238285
@DisplayName("should route non-event resources to scheduler even with event source enabled")
239286
void shouldRouteNonEventResourcesToScheduler() {
@@ -245,7 +292,7 @@ void shouldRouteNonEventResourcesToScheduler() {
245292

246293
new SimpliXStreamCoreConfiguration.StreamComponentWiring(
247294
subscriptionManager, schedulerManager, sessionManager, properties,
248-
collectorRegistry, eventStreamHandler, eventSourceRegistry);
295+
collectorRegistry, null, eventStreamHandler, eventSourceRegistry);
249296

250297
verify(subscriptionManager).setOnSubscriptionAdded(addCaptor.capture());
251298

simplix-stream/src/test/java/dev/simplecore/simplix/stream/core/session/SessionManagerTest.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,47 @@ void shouldNotInvokeCallbackWhenNotSet() {
633633
// No callback set
634634
assertDoesNotThrow(() -> sessionManager.terminateSession(session.getId()));
635635
}
636+
637+
@Test
638+
@DisplayName("should expose subscriptions to callback before clearing them")
639+
void shouldExposeSubscriptionsToCallbackBeforeClearing() {
640+
// Regression: callback used to fire AFTER clearSubscriptions(), so listeners
641+
// iterating session.getSubscriptions() saw an empty set and could not perform
642+
// per-subscription cleanup (e.g., removing scheduler subscribers).
643+
StreamSession session = StreamSession.create("user123", TransportType.SSE);
644+
SubscriptionKey k1 = SubscriptionKey.of("stock", Map.of("symbol", "AAPL"));
645+
SubscriptionKey k2 = SubscriptionKey.of("news", Map.of("topic", "tech"));
646+
session.addSubscription(k1);
647+
session.addSubscription(k2);
648+
when(sessionRegistry.findById(session.getId())).thenReturn(Optional.of(session));
649+
650+
AtomicReference<Set<SubscriptionKey>> snapshotInCallback = new AtomicReference<>();
651+
sessionManager.setOnSessionTerminated(s ->
652+
snapshotInCallback.set(s.getSubscriptions()));
653+
654+
sessionManager.terminateSession(session.getId());
655+
656+
assertNotNull(snapshotInCallback.get());
657+
assertEquals(Set.of(k1, k2), snapshotInCallback.get(),
658+
"callback must see subscriptions intact; reordering of clear/callback regressed");
659+
// After termination the session itself is cleared
660+
assertTrue(session.getSubscriptions().isEmpty());
661+
}
662+
663+
@Test
664+
@DisplayName("should still terminate when callback throws")
665+
void shouldStillTerminateWhenCallbackThrows() {
666+
StreamSession session = StreamSession.create("user123", TransportType.SSE);
667+
when(sessionRegistry.findById(session.getId())).thenReturn(Optional.of(session));
668+
669+
sessionManager.setOnSessionTerminated(s -> {
670+
throw new RuntimeException("listener boom");
671+
});
672+
673+
assertDoesNotThrow(() -> sessionManager.terminateSession(session.getId()));
674+
verify(sessionRegistry).unregister(session.getId());
675+
assertEquals(SessionState.TERMINATED, session.getState());
676+
}
636677
}
637678

638679
@Nested

0 commit comments

Comments
 (0)