Fix SessionFuture completion and ChannelFsm listener lifecycle bugs#1637
Merged
kevinherron merged 3 commits into1.0from Oct 31, 2025
Merged
Fix SessionFuture completion and ChannelFsm listener lifecycle bugs#1637kevinherron merged 3 commits into1.0from
kevinherron merged 3 commits into1.0from
Conversation
Complete pending SessionFuture instances when session creation is aborted via CloseSession events. The CreatingWait → Inactive and ReactivatingWait → Closing transitions now call handleFailureToOpenSession() to ensure any GetSession or OpenSession futures are properly completed with Bad_SessionClosed exceptions. Prevent ChannelFsm listener accumulation when cycling through Active state. Multiple listeners could accumulate if keep-alive timeouts triggered reactivation without actual channel disconnection. Listeners are now stored in FSM context and explicitly removed when transitioning away from Active state, ensuring exactly one listener is registered during each Active period.
Add testCloseSessionCompletesSessionFutureInReactivatingWait to verify that pending SessionFuture instances are properly completed with an exception when closeSession() is called during ReactivatingWait state. This covers the ReactivatingWait → Closing transition behavior where session futures must be completed before state transition completes. Make State enum public to allow test access. Add javadoc to OpcUaClient.getSessionFsm() and SessionFsm.getState().
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR addresses issues with SessionFuture completion and ChannelFsm listener cleanup in the OPC UA session management. The changes ensure proper exception handling when sessions are closed during wait states and fix resource leaks from transition listeners.
Key changes:
- Added calls to
handleFailureToOpenSessioninCreatingWaitandReactivatingWaitstates to properly complete session futures with exceptions when closing - Improved ChannelFsm transition listener lifecycle management by storing listener references and removing them on all transitions out of Active state
- Made the
Stateenum public and addedgetState()andgetSessionFsm()methods to enable external state inspection
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| State.java | Changed enum visibility from package-private to public to enable external access |
| SessionFsm.java | Added getState() method and KEY_CHANNEL_FSM_TRANSITION_LISTENER context key for listener lifecycle tracking |
| SessionFsmFactory.java | Fixed listener cleanup, added session future completion on close in wait states, broadened transition condition |
| OpcUaClient.java | Added getSessionFsm() method to expose SessionFsm for state inspection |
| SessionFsmTest.java | Added test to verify SessionFuture completion when closing in ReactivatingWait state |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...dk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/session/SessionFsmFactory.java
Show resolved
Hide resolved
...ntegration-tests/src/test/java/org/eclipse/milo/opcua/sdk/client/session/SessionFsmTest.java
Show resolved
Hide resolved
...ntegration-tests/src/test/java/org/eclipse/milo/opcua/sdk/client/session/SessionFsmTest.java
Show resolved
Hide resolved
ia-cmorgan
approved these changes
Oct 31, 2025
There was a problem hiding this comment.
Should the copyright in this file be updated?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Complete pending SessionFuture instances when session creation is aborted via CloseSession events. The CreatingWait → Inactive and ReactivatingWait → Closing transitions now call handleFailureToOpenSession() to ensure any GetSession or OpenSession futures are properly completed with Bad_SessionClosed exceptions.
Prevent ChannelFsm listener accumulation when cycling through Active state. Multiple listeners could accumulate if keep-alive timeouts triggered reactivation without actual channel disconnection. Listeners are now stored in FSM context and explicitly removed when transitioning away from Active state, ensuring exactly one listener is registered during each Active period.