[kevinherron]

Fixes #2134.

Problem

OpcTcpClientTransport.disconnect() can take 5 seconds when the connection closes at about the same moment the application calls disconnect(). For example, the server drops the connection while the client is shutting down.

The ChannelFsm disconnect action completed its future from a channelInactive handler that it added to the front of the pipeline. The action runs on the transport executor, not the event loop, so the handler can miss the event:

• If the channel is already closed, it has been deregistered, and Netty never calls the new handler.
• If the channel is closing, Netty skips a handler whose handlerAdded hasn't run on the event loop yet.

The action sees a closed channel when the ChannelFsm evaluates Disconnect before the ChannelInactive event that the close queued. disconnect() then waited for the 5 second fallback timeout.

Fix

The action now completes the disconnect from the channel's close future. Netty notifies a close future listener even when it's added after the channel has closed.

-      channel.pipeline().addFirst(new ChannelInboundHandlerAdapter() {
-        @Override
-        public void channelInactive(ChannelHandlerContext ctx) throws Exception {
-          timeout.cancel();
-          disconnectFuture.complete(null);
-          super.channelInactive(ctx);
-        }
-      });
+      channel.closeFuture().addListener(f -> {
+        timeout.cancel();
+        disconnectFuture.complete(null);
+      });

The rest of the flow is unchanged. The action still sends CloseSecureChannel, and the 5 second timeout still closes the channel if the server never does. The timeout now only closes the channel, and the close future listener completes the disconnect in both cases. The listener runs when the channel closes, before channelInactive reaches the UASC handlers, so the disconnect still completes first, as it did when the handler sat at the front of the pipeline.

OpcWebSocketClientTransport had the same disconnect action, so it gets the same change.

netty-channel-fsm 1.0.4

The second commit updates netty-channel-fsm from 1.0.2 to 1.0.4. It is separate from the fix above. The regression test still fails on 1.0.4 without the Milo change.

1.0.3 closes a channel from a connection attempt that is still pending when disconnect() is requested, instead of leaking it. 1.0.4 fixes cases where the FSM could get stuck, leak a channel, or end in the wrong state:

• Events from a previous channel, such as a late close, no longer affect a newer connection.
• A channel that closes before ConnectSuccess is evaluated is detected, so the FSM reconnects instead of staying Connected with a closed channel.
• connect() and disconnect() requests apply in the order they were made. Before, disconnect(), connect(), disconnect() could end Connected and leave a client reconnecting forever.
• A ChannelActions method that throws or returns null no longer leaves the FSM stuck.

Milo needs no code changes for it. The public API it uses is unchanged. The 1.0.4 behavior changes that touch Milo are harmless:

• getChannel() returns a copy of the FSM's future. The transport already copied it before sending requests.
• Transition listeners see a Disconnecting to Disconnecting transition after every close. The transport's listener only reports transitions into or out of Connected, so it ignores them.
• A getChannel() made after a pending disconnect() fails with "not connected" instead of returning the channel that is about to close.

Tests

Two tests in OpcTcpTransportTest run against a real server transport and bound disconnect() to 2 seconds, well inside the 5 second fallback, so passing shows the channel close completed the disconnect.

• disconnectCompletesWhenChannelClosedBeforeDisconnectActionRuns gives the transport a single-thread executor and blocks it, calls disconnect(), closes the channel, then releases the executor. This makes the ChannelFsm run the disconnect action against a closed channel. It timed out on the unfixed code with both netty-channel-fsm 1.0.2 and 1.0.4, and passes in about 3 ms with the fix.
• disconnectSendsCloseSecureChannelAndCompletesWhenChannelCloses covers the normal path. It watches the raw outbound bytes for a CLO message and checks that the channel is closed when disconnect() completes.

The WebSocket transport has no test suite, so its copy of the change is covered by compilation only.

Verification:

• mvn -q spotless:apply and mvn -q clean compile
• mvn -q -pl opc-ua-stack/transport -am test with the fix on 1.0.2: 1669 stack-core and 167 transport tests, no failures
• mvn -q clean verify with the fix on 1.0.4: all modules pass, including 1203 integration tests and 214 sdk-client tests

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable defects were found.

Summary

Completes TCP and WebSocket disconnects from the channel’s close future, including when the channel has already closed.

• TCP and WebSocket disconnects finish as soon as their channel closes.
• The client state machine uses netty-channel-fsm 1.0.4.

No actionable issues were found. Build and test results were not independently rerun.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Disconnect action] --> B[Schedule five-second forced close]
  B --> C[Listen to channel close future]
  C --> D[Send CloseSecureChannel]
  D --> E[Channel closes]
  B -->|Timeout if needed| E
  E --> F[Close listener cancels timeout]
  F --> G[Complete disconnect]
  C -->|Channel already closed| F

Loading

_{Reviews (1) · Last reviewed commit: "Update netty-channel-fsm to 1.0.4" · Reviewed by Greptile}

[kevinherron]

/request-license-review

[github-actions]

/request-license-review

⚠️ Failed to request review of not vetted licenses.

Workflow run (with attached summary files):
https://github.com/eclipse-milo/milo/actions/runs/39999337653