Backport: Honour consoleproxy.session.timeout for noVNC console sessions by dheeraj12347 · Pull Request #13058 · apache/cloudstack

dheeraj12347 · 2026-04-22T18:49:34Z

Description

This PR backports the console proxy/noVNC timeout fix to the 4.20 branch.

It ensures that consoleproxy.session.timeout is honoured for noVNC console
sessions, so idle sessions are cleaned up correctly and do not linger
indefinitely.

Key points:

Wire consoleproxy.session.timeout through the console proxy server for
noVNC-based console sessions.
Apply the timeout in the WebSocket handler and GC thread so idle sessions
are closed after the configured period.
Keep the change minimal and compatible with 4.20: do not introduce the
newer sessionRequiresNewViewer API on ConsoleProxyClientParam, only
remove the references that exist in later branches.

Related work

Backport of the console proxy timeout fix from the main branch.
Intended to address the same behaviour as the upstream change that fixed
idle noVNC sessions not respecting consoleproxy.session.timeout.

Testing

Compiled the console proxy server module successfully:

mvn -pl services/console-proxy/server -am -DskipTests compile

Verified that the code builds cleanly with checkstyle on the 4.20 branch.
Fix Global setting "consoleproxy.session.timeout " is not honoured #12810

Copilot

Pull request overview

This PR aims to backport the fix to ensure consoleproxy.session.timeout is honored for noVNC console sessions on the 4.20 branch, so idle sessions are cleaned up and don’t linger indefinitely.

Changes:

Updates the noVNC WebSocket handler with additional logging, parameter validation, and safer frame/error handling.
Refactors the console proxy GC thread loop and related logging around idle session cleanup.
Adjusts console proxy startup/authentication reflection and noVNC viewer creation/replacement logic.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java	WebSocket connect/frame/error handling changes intended to support correct idle-session cleanup.
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java	GC loop refactor and idle session timeout constant/comment updates.
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java	Console proxy startup/auth reflection changes and noVNC viewer lifecycle adjustments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-28T10:04:18Z

                result =
-                        authMethod.invoke(ConsoleProxy.context, param.getClientHostAddress(), String.valueOf(param.getClientHostPort()), param.getClientTag(),
-                                param.getClientHostPassword(), param.getTicket(), reauthentication, param.getSessionUuid());
+                        authMethod.invoke(ConsoleProxy.context, param.getClientHostAddress(), String.valueOf(param.getClientHostPort()),
+                                param.getClientTag(), param.getClientHostPassword(), param.getTicket(), reauthentication,
+                                param.getSessionUuid(), param.getClientIp());
            } catch (IllegalAccessException e) {


authenticateConsoleAccess(...) now invokes authMethod with an extra param.getClientIp() argument. On 4.20, ConsoleProxyResource.authenticateConsoleAccess(...) only accepts 7 arguments, so this call will fail if/when authMethod is resolved. Keep the invocation arguments consistent with the reflected method signature (or implement a dual-signature invocation path).

Copilot · 2026-04-28T10:04:19Z

        String websocketUrl = queryMap.get("websocketUrl");
        String sessionUuid = queryMap.get("sessionUuid");
        String clientIp = session.getRemoteAddress().getAddress().getHostAddress();
+        boolean sessionRequiresNewViewer = Boolean.parseBoolean(queryMap.get("sessionRequiresNewViewer"));


sessionRequiresNewViewer is parsed from the query string but never used. If this branch intentionally does not support that behavior (per the PR description), remove this local variable (and possibly the query param handling) to avoid confusion and dead code.

Suggested change

boolean sessionRequiresNewViewer = Boolean.parseBoolean(queryMap.get("sessionRequiresNewViewer"));

Copilot · 2026-04-28T10:04:19Z

+        if (viewer == null) {
+            logger.debug("Ignoring WebSocket frame because viewer is not initialized yet.");
+            return;
+        }


NoVNC session idle tracking relies on getClientLastFrontEndActivityTime(), but this handler does not update the viewer's front-end activity timestamp when frames arrive from the browser. As a result, GC/linger logic may not reflect actual browser activity and can prevent consoleproxy.session.timeout from being enforced. Update the viewer's front-end activity time on each received WebSocket frame (and ensure the timestamp isn't continuously refreshed by backend reads alone).

Suggested change

}

}

viewer.setClientLastFrontEndActivityTime(System.currentTimeMillis());

codecov · 2026-04-28T15:02:46Z

Codecov Report

❌ Patch coverage is 2.08333% with 94 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.26%. Comparing base (549daae) to head (2438319).

Files with missing lines	Patch %	Lines
...main/java/com/cloud/consoleproxy/ConsoleProxy.java	2.94%	66 Missing ⚠️
...m/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java	0.00%	15 Missing ⚠️
...a/com/cloud/consoleproxy/ConsoleProxyGCThread.java	0.00%	13 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.20   #13058      +/-   ##
============================================
- Coverage     16.26%   16.26%   -0.01%     
- Complexity    13434    13435       +1     
============================================
  Files          5667     5667              
  Lines        500731   500760      +29     
  Branches      60803    60817      +14     
============================================
- Hits          81455    81453       -2     
- Misses       410172   410203      +31     
  Partials       9104     9104

Flag	Coverage Δ
uitests	`4.14% <ø> (ø)`
unittests	`17.12% <2.08%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dheeraj12347 · 2026-04-29T22:34:47Z

Thanks for the detailed review and suggestions from Copilot.
I’ve read and understood all of the comments, especially around the 4.20 authenticateConsoleAccess signature/invocation, idle timeout handling in the GC thread, noVNC viewer activity tracking, and the unused sessionRequiresNewViewer flag.
I’ll work through each point and push updates to address them as soon as possible.

DaanHoogland · 2026-05-06T15:35:19Z

@dheeraj12347 , I set consoleproxy.session.timeout to 30000, expecting my consoles to get lost in half a minute. I restarted the MS. The consoles keep working until about the half an hour, the default setting. (I didn’t measure exactly) Can you have another look at this?

weizhouapache · 2026-05-07T14:20:55Z

@dheeraj12347
only one of #13002 and #13058 is needed.

once this is merged into 4.20, it will be included in main as well after forward-merge

dheeraj12347 · 2026-05-09T11:10:47Z

@dheeraj12347 , I set consoleproxy.session.timeout to 30000, expecting my consoles to get lost in half a minute. I restarted the MS. The consoles keep working until about the half an hour, the default setting. (I didn’t measure exactly) Can you have another look at this?

Hi @DaanHoogland ,

I’ve updated the PR to address the consoleproxy.session.timeout behaviour you described:

In ConsoleProxy.java I now read consoleproxy.session.timeout from the configuration (milliseconds), validate it, and store the effective value in ConsoleProxy.sessionTimeoutMillis, with a log line on startup showing the effective timeout.

In ConsoleProxyGCThread.java I removed the hardcoded idle timeout and changed the GC logic to compute each viewer’s idle time in milliseconds and compare it against ConsoleProxy.sessionTimeoutMillis, logging the idle duration and configured timeout when a session is removed.

To verify my changes, I built the console proxy server module with:

bash
mvn -pl services/console-proxy/server -am clean compile

I’m open to any feedback on this approach, and I’m happy to further adjust or iterate based on your results when you re-test with consoleproxy.session.timeout=30000.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

        String sessionUuid = queryMap.get("sessionUuid");
        String clientIp = session.getRemoteAddress().getAddress().getHostAddress();
+        boolean sessionRequiresNewViewer = Boolean.parseBoolean(queryMap.get("sessionRequiresNewViewer"));

-        if (tag == null)
+        if (tag == null) {
            tag = "";
+        }


+            Set<String> keys = connMap.keySet();
+            Iterator<String> iterator = keys.iterator();
            while (iterator.hasNext()) {
                String key;
                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;
                }

+
                synchronized (connMap) {
                    connMap.remove(key);
                    bReportLoad = true;
                }


    public static final int VIEWER_LINGER_SECONDS = 180;

+    // New: default and effective session timeout (milliseconds) honoured from consoleproxy.session.timeout
+    public static final int DEFAULT_SESSION_TIMEOUT_MILLIS = 300000;
+    public static volatile int sessionTimeoutMillis = DEFAULT_SESSION_TIMEOUT_MILLIS;
+
+


DaanHoogland · 2026-08-04T07:59:19Z

@blueorangutan package

blueorangutan · 2026-08-04T08:00:05Z

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2026-08-04T09:24:26Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18758

DaanHoogland · 2026-08-04T09:25:43Z

@blueorangutan test

blueorangutan · 2026-08-04T09:28:07Z

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

boring-cyborg Bot added the component:console-proxy label Apr 22, 2026

DaanHoogland requested a review from Copilot April 28, 2026 09:54

DaanHoogland linked an issue Apr 28, 2026 that may be closed by this pull request

Global setting "consoleproxy.session.timeout " is not honoured #12810

Open

Copilot started reviewing on behalf of DaanHoogland April 28, 2026 09:56 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

DaanHoogland modified the milestones: 4.22.2, 4.20.4 May 13, 2026

DaanHoogland added this to Apache CloudStack BugFest - Issues May 13, 2026

DaanHoogland removed this from Apache CloudStack BugFest - Issues May 13, 2026

DaanHoogland requested a review from Copilot May 21, 2026 14:38

Copilot started reviewing on behalf of DaanHoogland May 21, 2026 14:38 View session

This comment was marked as low quality.

Sign in to view

DaanHoogland requested a review from Copilot May 26, 2026 09:55

Copilot started reviewing on behalf of DaanHoogland May 26, 2026 09:55 View session

This comment was marked as low quality.

Sign in to view

DaanHoogland requested a review from Copilot May 27, 2026 08:21

Copilot started reviewing on behalf of DaanHoogland May 27, 2026 08:21 View session

Copilot AI reviewed May 27, 2026

View reviewed changes

apache deleted a comment from blueorangutan May 27, 2026

weizhouapache mentioned this pull request Jun 30, 2026

Honour consoleproxy.session.timeout for noVNC sessions (fixes #12810) #13002

Closed

dheeraj12347 added 2 commits August 4, 2026 09:58

Fix console proxy idle timeout and noVNC session handling

6f7bf55

Honor consoleproxy.session.timeout in console proxy GC

2438319

DaanHoogland force-pushed the backport-12810-consoleproxy-timeout-4.20 branch from 527fe0e to 2438319 Compare August 4, 2026 07:58

	}
	}
	viewer.setClientLastFrontEndActivityTime(System.currentTimeMillis());

Uh oh!

Conversation

dheeraj12347 commented Apr 22, 2026

Description

Related work

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dheeraj12347 commented Apr 29, 2026

Uh oh!

DaanHoogland commented May 6, 2026

Uh oh!

weizhouapache commented May 7, 2026

Uh oh!

dheeraj12347 commented May 9, 2026

Uh oh!

This comment was marked as low quality.

Uh oh!

This comment was marked as low quality.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

DaanHoogland commented Aug 4, 2026

Uh oh!

blueorangutan commented Aug 4, 2026

Uh oh!

blueorangutan commented Aug 4, 2026

Uh oh!

DaanHoogland commented Aug 4, 2026

Uh oh!

blueorangutan commented Aug 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Apr 28, 2026 •

edited

Loading