This is an automated email from the ASF dual-hosted git repository. bonampak pushed a commit to branch feature/jakarta-jetty-upgrade in repository https://gitbox.apache.org/repos/asf/knox.git
commit 469fcbf8edaedf1a7fc3d5a12f77e0a8efe05a80 Author: bonampak <[email protected]> AuthorDate: Wed Aug 5 19:03:07 2026 +0200 KNOX-3238: added TODO comments on websocket issues still present. --- .../apache/knox/gateway/webshell/WebshellWebSocketAdapter.java | 5 +++++ .../apache/knox/gateway/websockets/ProxyWebSocketAdapter.java | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/webshell/WebshellWebSocketAdapter.java b/gateway-server/src/main/java/org/apache/knox/gateway/webshell/WebshellWebSocketAdapter.java index 5fb439c1c..2939e9fa2 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/webshell/WebshellWebSocketAdapter.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/webshell/WebshellWebSocketAdapter.java @@ -119,6 +119,11 @@ public class WebshellWebSocketAdapter extends ProxyWebSocketAdapter { cleanup(); return; } + // TODO (KNOX-3238): The failure lambda runs on a Jetty I/O thread while blockingReadFromHost + // runs on the pool thread — both can call cleanup() concurrently, racing on the session field. + // The fix is to use manual demand management (Session.Listener) so that demand() is called only + // from the send callback, serializing sends and eliminating the cross-thread race. + // See also the class-level TODO in ProxyWebSocketAdapter. session.sendText(message, Callback.from( () -> { /* success: nothing to do */ }, t -> { diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/websockets/ProxyWebSocketAdapter.java b/gateway-server/src/main/java/org/apache/knox/gateway/websockets/ProxyWebSocketAdapter.java index 039d454fc..4d7449669 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/websockets/ProxyWebSocketAdapter.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/websockets/ProxyWebSocketAdapter.java @@ -52,6 +52,13 @@ import org.eclipse.jetty.websocket.api.StatusCode; * * @since 0.10 */ +// TODO (KNOX-3238): ProxyWebSocketAdapter uses AbstractAutoDemanding which auto-calls demand() after +// each onWebSocketText/onWebSocketPong returns, meaning the next backend message can be dispatched +// before the previous frontendSession.sendText/sendPing callback has fired. This violates the Jetty 12 +// WebSocket API contract ("you cannot initiate another send until the previous send is completed"). +// The fix is to switch to manual demand management (Session.Listener) and call session.demand() only +// from the send callback's success path, mirroring the pattern in Jetty's own WebSocketProxy: +// jetty.project/jetty-core/jetty-websocket/jetty-websocket-jetty-tests/.../proxy/WebSocketProxy.java public class ProxyWebSocketAdapter extends Session.Listener.AbstractAutoDemanding { protected static final WebsocketLogMessages LOG = MessagesFactory.get(WebsocketLogMessages.class); @@ -281,6 +288,8 @@ public class ProxyWebSocketAdapter extends Session.Listener.AbstractAutoDemandin flushBufferedMessages(); LOG.debugLog("Sending current message [From Backend <---]: " + message); + // TODO (KNOX-3238): Callback.NOOP silently discards send failures — restoring error handling + // requires switching to manual demand management first (see class-level TODO). frontendSession.sendText(message, Callback.NOOP); } finally { remoteLock.unlock(); @@ -307,6 +316,7 @@ public class ProxyWebSocketAdapter extends Session.Listener.AbstractAutoDemandin flushBufferedMessages(); LOG.logMessage("Sending current PING [From Backend <---]: "); + // TODO (KNOX-3238): same as above — Callback.NOOP loses send failures. frontendSession.sendPing(message.getApplicationData(), Callback.NOOP); } finally { remoteLock.unlock();
