Backport: Honour consoleproxy.session.timeout for noVNC console sessions

server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java

@@ -1259,6 +1259,7 @@ public boolean finalizeVirtualMachineProfile(VirtualMachineProfile profile, Depl
         buf.append(" pod=").append(dest.getPod().getId());
         buf.append(" guid=Proxy.").append(profile.getId());
         buf.append(" proxy_vm=").append(profile.getId());
+        buf.append(" consoleproxy.session.timeout=").append(proxySessionTimeoutValue);
         if (disableRpFilter) {
             buf.append(" disable_rp_filter=true");
         }

services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java

@@ -18,7 +18,7 @@
 
 import java.io.File;
 import java.util.ArrayList;
-import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -32,9 +32,8 @@
  * management software
  */
 public class ConsoleProxyGCThread extends Thread {
-    protected Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
+    private static final Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
 
-    private final static int MAX_SESSION_IDLE_SECONDS = 180;
 
     private final Map<String, ConsoleProxyClient> connMap;
     private final Set<String> removedSessionsSet;
@@ -46,21 +45,20 @@ public ConsoleProxyGCThread(Map<String, ConsoleProxyClient> connMap, Set<String>
     }
 
     private void cleanupLogging() {
-        if (lastLogScan != 0 && System.currentTimeMillis() - lastLogScan < 3600000)
+        if (lastLogScan != 0 && System.currentTimeMillis() - lastLogScan < 3600000) {
             return;
-
+        }
         lastLogScan = System.currentTimeMillis();
 
         File logDir = new File("./logs");
-        File files[] = logDir.listFiles();
+        File[] files = logDir.listFiles();
         if (files != null) {
             for (File file : files) {
                 if (System.currentTimeMillis() - file.lastModified() >= 86400000L) {
                     try {
                         file.delete();
                     } catch (Throwable e) {
-                        logger.info("[ignored]"
-                                + "failed to delete file: " + e.getLocalizedMessage());
+                        logger.info("[ignored] failed to delete file: " + e.getLocalizedMessage());
                     }
                 }
             }
@@ -78,21 +76,25 @@ public void run() {
             bReportLoad = false;
 
             if (logger.isDebugEnabled()) {
-                logger.debug(String.format("connMap=%s, removedSessions=%s", connMap, removedSessionsSet));
+                logger.debug(String.format("ConsoleProxyGCThread loop: connMap=%s, removedSessions=%s", connMap, removedSessionsSet));
             }
-            Set<String> e = connMap.keySet();
-            Iterator<String> iterator = e.iterator();
-            while (iterator.hasNext()) {
-                String key;
+            List<String> keys;
+            synchronized (connMap) {
+                 keys = new ArrayList<>(connMap.keySet());
+             }
+             for (String key : keys) {
                 ConsoleProxyClient client;
 
                 synchronized (connMap) {
-                    key = iterator.next();
                     client = connMap.get(key);
                 }
 
-                long seconds_unused = (System.currentTimeMillis() - client.getClientLastFrontEndActivityTime()) / 1000;
-                if (seconds_unused < MAX_SESSION_IDLE_SECONDS) {
+                if (client == null) {
+                    continue;
+                }
+
+                long millisecondsUnused = System.currentTimeMillis() - client.getClientLastFrontEndActivityTime();
+                if (millisecondsUnused < ConsoleProxy.sessionTimeoutMillis) {
                     continue;
                 }
 
@@ -102,12 +104,13 @@ public void run() {
                 }
 
                 // close the server connection
-                logger.info("Dropping " + client + " which has not been used for " + seconds_unused + " seconds");
+                logger.info("Dropping " + client + " which has not been used for " + millisecondsUnused
+                        + " ms (configured timeout: " + ConsoleProxy.sessionTimeoutMillis + " ms)");
                 client.closeClient();
             }
 
             if (bReportLoad || System.currentTimeMillis() - lastReportTick > 5000) {
-                // report load changes
+                // report load changes, including removed sessions since last report
                 ConsoleProxyClientStatsCollector collector = new ConsoleProxyClientStatsCollector(connMap);
                 collector.setRemovedSessions(new ArrayList<>(removedSessionsSet));
                 String loadInfo = collector.getStatsReport();
@@ -125,7 +128,7 @@ public void run() {
             try {
                 Thread.sleep(5000);
             } catch (InterruptedException ex) {
-                logger.debug("[ignored] Console proxy was interrupted during GC.");
+                logger.debug("[ignored] Console proxy GC thread interrupted.", ex);
             }
         }
     }

services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java

@@ -17,6 +17,8 @@
 package com.cloud.consoleproxy;
 
 import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
 import java.util.Map;
 
 import javax.servlet.ServletException;
@@ -34,14 +36,18 @@
 import org.eclipse.jetty.websocket.api.annotations.OnWebSocketFrame;
 import org.eclipse.jetty.websocket.api.annotations.WebSocket;
 import org.eclipse.jetty.websocket.api.extensions.Frame;
+import org.eclipse.jetty.websocket.api.extensions.Frame.Type;
 import org.eclipse.jetty.websocket.server.WebSocketHandler;
 import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
 
 @WebSocket
 public class ConsoleProxyNoVNCHandler extends WebSocketHandler {
 
+    private static final Logger logger = LogManager.getLogger(ConsoleProxyNoVNCHandler.class);
+
+    private static final String ACTIVITY_SIGNAL = "CLOUDSTACK_ACTIVITY";
+
     private ConsoleProxyNoVncClient viewer = null;
-    protected Logger logger = LogManager.getLogger(getClass());
 
     public ConsoleProxyNoVNCHandler() {
         super();
@@ -73,6 +79,10 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
 
     @OnWebSocketConnect
     public void onConnect(final Session session) throws IOException, InterruptedException {
+        session.setIdleTimeout(ConsoleProxy.sessionTimeoutMillis);
+        logger.debug("Set WebSocket idle timeout to {} ms for session from {}.",
+                ConsoleProxy.sessionTimeoutMillis, session.getRemoteAddress());
+
         String queries = session.getUpgradeRequest().getQueryString();
         Map<String, String> queryMap = ConsoleProxyHttpHandlerHelper.getQueryMap(queries);
 
@@ -94,14 +104,14 @@ public void onConnect(final Session session) throws IOException, InterruptedExce
         String sessionUuid = queryMap.get("sessionUuid");
         String clientIp = session.getRemoteAddress().getAddress().getHostAddress();
 
-        if (tag == null)
+        if (tag == null) {
             tag = "";
+        }
 
-        long ajaxSessionId = 0;
         int port;
-
-        if (host == null || portStr == null || sid == null)
-            throw new IllegalArgumentException();
+        if (host == null || portStr == null || sid == null) {
+            throw new IllegalArgumentException("Missing required console connection parameters");
+        }
 
         try {
             port = Integer.parseInt(portStr);
@@ -112,7 +122,7 @@ public void onConnect(final Session session) throws IOException, InterruptedExce
 
         if (ajaxSessionIdStr != null) {
             try {
-                ajaxSessionId = Long.parseLong(ajaxSessionIdStr);
+                Long.parseLong(ajaxSessionIdStr);
             } catch (NumberFormatException e) {
                 logger.error("Invalid ajaxSessionId (sess) value in query string: {}. Expected a number.", ajaxSessionIdStr, e);
                 throw new IllegalArgumentException(e);
@@ -151,7 +161,7 @@ public void onConnect(final Session session) throws IOException, InterruptedExce
             viewer = ConsoleProxy.getNoVncViewer(param, ajaxSessionIdStr, session);
             logger.info("Viewer has been created successfully [session UUID: {}, client IP: {}].", sessionUuid, clientIp);
         } catch (Exception e) {
-            logger.error("Failed to create viewer [session UUID: {}, client IP: {}] due to {}.", sessionUuid, clientIp, e.getMessage(), e);
+            logger.error("Failed to create viewer [session UUID: {}, client IP: {}].", sessionUuid, clientIp, e);
             return;
         } finally {
             if (viewer == null) {
@@ -160,7 +170,7 @@ public void onConnect(final Session session) throws IOException, InterruptedExce
         }
     }
 
-    private boolean checkSessionSourceIp(final Session session, final String sourceIP, String sessionSourceIP) throws IOException {
+    private boolean checkSessionSourceIp(final Session session, final String sourceIP, final String sessionSourceIP) throws IOException {
         logger.info("Verifying session source IP {} from WebSocket connection request.", sessionSourceIP);
         if (ConsoleProxy.isSourceIpCheckEnabled && (sessionSourceIP == null || !sessionSourceIP.equals(sourceIP))) {
             logger.warn("Failed to access console as the source IP to request the console is {}.", sourceIP);
@@ -174,7 +184,7 @@ private boolean checkSessionSourceIp(final Session session, final String sourceI
     @OnWebSocketClose
     public void onClose(Session session, int statusCode, String reason) throws IOException, InterruptedException {
         String sessionSourceIp = session.getRemoteAddress().getAddress().getHostAddress();
-        logger.debug("Closing WebSocket session [source IP: {}, status code: {}].", sessionSourceIp, statusCode);
+        logger.debug("Closing WebSocket session [source IP: {}, status code: {}, reason: {}].", sessionSourceIp, statusCode, reason);
         if (viewer != null) {
             ConsoleProxy.removeViewer(viewer);
         }
@@ -183,12 +193,40 @@ public void onClose(Session session, int statusCode, String reason) throws IOExc
 
     @OnWebSocketFrame
     public void onFrame(Frame f) throws IOException {
-        logger.trace("Sending client [ID: {}] frame of {} bytes.", viewer.getClientId(), f.getPayloadLength());
+        if (viewer == null) {
+            logger.debug("Ignoring WebSocket frame because viewer is not initialized yet.");
+            return;
+        }
+
+        if (f.getType() == Type.TEXT) {
+            ByteBuffer payload = f.getPayload().slice();
+            byte[] data = new byte[payload.remaining()];
+            payload.get(data);
+
+            String message = new String(data, StandardCharsets.UTF_8);
+            if (ACTIVITY_SIGNAL.equals(message)) {
+                viewer.updateFrontEndActivityTime();
+            } else {
+                logger.debug("Ignoring unexpected text WebSocket frame.");
+            }
+            return;
+        }
+
+        if (f.getType() != Type.BINARY) {
+            return;
+        }
+
+        logger.trace("Sending client [ID: {}] frame of {} bytes.",
+                viewer.getClientId(), f.getPayloadLength());
         viewer.sendClientFrame(f);
     }
 
     @OnWebSocketError
     public void onError(Throwable cause) {
-        logger.error("Error on WebSocket [client ID: {}, session UUID: {}].", cause, viewer.getClientId(), viewer.getSessionUuid());
+        if (viewer != null) {
+            logger.error("Error on WebSocket [client ID: {}, session UUID: {}].", viewer.getClientId(), viewer.getSessionUuid(), cause);
+        } else {
+            logger.error("Error on WebSocket before viewer initialization.", cause);
+        }
     }
 }

services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java

@@ -118,17 +118,13 @@ public void run() {
                     while (connectionAlive) {
                         logger.trace("Connection with client [{}] [IP: {}] is alive.", clientId, clientSourceIp);
                         if (client.isVncOverWebSocketConnection()) {
-                            if (client.isVncOverWebSocketConnectionOpen()) {
-                                updateFrontEndActivityTime();
-                            }
                             connectionAlive = session.isOpen();
                             sleepTime = 1;
                         } else if (client.isVncOverNioSocket()) {
                             ByteBuffer buffer = getOrCreateReadBuffer();
                             int bytesRead = client.readAvailableDataIntoBuffer(buffer, buffer.remaining());
 
                             if (bytesRead > 0) {
-                                updateFrontEndActivityTime();
                                 consecutiveZeroReads = 0; // Reset counter on successful read
 
                                 sleepTime = 0; // Still no sleep to catch any remaining data quickly
@@ -185,7 +181,6 @@ private boolean sendReadBytesToNoVNC(ByteBuffer buffer) {
         try {
             // Buffer is already prepared for reading by flip()
             session.getRemote().sendBytes(buffer);
-            updateFrontEndActivityTime();
         } catch (WebSocketException | IOException e) {
             logger.error("VNC server connection exception.", e);
             return false;

systemvm/agent/noVNC/core/rfb.js

@@ -516,6 +516,8 @@ export default class RFB extends EventTargetMixin {
             return;
         }
 
+        this._sock.sendActivity();
+
         const scancode = XtScancode[code];
 
         if (keysym === KeyTable.XK_Shift_L || keysym === KeyTable.XK_Shift_R) {
@@ -1210,6 +1212,8 @@ export default class RFB extends EventTargetMixin {
         if (this._rfbConnectionState !== 'connected') { return; }
         if (this._viewOnly) { return; } // View only, skip mouse events
 
+        this._sock.sendActivity();
+
         RFB.messages.pointerEvent(this._sock, this._display.absX(x),
                                   this._display.absY(y), mask);
     }

systemvm/agent/noVNC/core/websock.js

@@ -203,6 +203,21 @@ export default class Websock {
         this.flush();
     }
 
+    sendActivity() {
+        if (this._websocket === null || this.readyState !== 'open') {
+            return;
+        }
+
+        const now = Date.now();
+        if (this._lastActivitySignal &&
+            now - this._lastActivitySignal < 500) {
+            return;
+        }
+
+        this._lastActivitySignal = now;
+        this._websocket.send("CLOUDSTACK_ACTIVITY");
+    }
+
     sendString(str) {
         this.send(str.split('').map(chr => chr.charCodeAt(0)));
     }