davsclaus commented on code in PR #27764:
URL: https://github.com/apache/camel/pull/27764#discussion_r4257839069


##########
components/camel-vertx/camel-vertx-websocket/src/main/java/org/apache/camel/component/vertx/websocket/VertxWebsocketEndpoint.java:
##########
@@ -195,14 +224,13 @@ protected WebSocket 
getWebSocket(java.util.function.Consumer<WebSocket> setupCal
                 if (result.succeeded()) {
                     WebSocket ws = result.result();
                     if (future.isDone()) {
-                        // the caller gave up waiting (see below): nobody 
would use or close this WebSocket
                         ws.close();

Review Comment:
   Nit: the dedup commit dropped the comments here (`the caller gave up waiting 
(see below): nobody would use or close this WebSocket`) and at 
`future.cancel(false)` (`a connect that completes from now on closes its 
WebSocket`). They came from #27762 and explain why the late connect is closed. 
Could you keep them?



##########
components/camel-vertx/camel-vertx-websocket/src/main/java/org/apache/camel/component/vertx/websocket/VertxWebsocketEndpoint.java:
##########
@@ -163,6 +164,20 @@ Lock getClientConsumerConnectLock() {
         return clientConsumerConnectLock;
     }
 
+    /**
+     * Clears the message, close, and exception handlers on the endpoint's 
WebSocket, but only if it is still open and
+     * {@code owner} is the last consumer that registered handlers on it. The 
caller must hold
+     * {@link #clientConsumerConnectLock}.
+     */
+    void clearWebSocketHandlers(WebSocket owner) {

Review Comment:
   This checks socket identity, not handler ownership. Two routes that consume 
the same `consumeAsClient` URI share this endpoint and its WebSocket. Route A 
starts and registers its handlers. Route B starts, finds the socket open and 
(with this PR) registers its handlers over A's. Now stop route A: 
`A.handlerWebSocket == webSocket`, so A clears **B's** handlers, and route B 
silently stops receiving messages and will not reconnect, because its 
`closeHandler` is gone too.
   
   Could the endpoint record which consumer registered the current handlers? 
For example, set a `handlerOwner` field in the setup callback (it already runs 
under `clientConsumerConnectLock`). Then `clearWebSocketHandlers(consumer)` 
would detach only when `handlerOwner == consumer`. A second test with two 
client-consumer routes on the same URI (stop one, the other still receives) 
would cover it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to