From 0db844f33620840942dff6ee352e64a729095232 Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Mon, 30 Jan 2023 08:19:48 -0300 Subject: [PATCH] Backport community fix for #210 (#222) * Create table to store available console sessions * Handle the new table in Java (VO and DAO) * Manage console sessions via database * Fix cherry-pick: verify in database if the session exist * Address reviews: rename table to console_session and rename java objects according to the table name * Redesign console_session to store more data * Remove unnecessary constructor * Use create table syntax previous to mariadb 10.5 * Add console session cleanup task * Address review * Add missing config keys * Fix sonar cloud reports * In progress fix console load report * Fix remove console when session ends * Improve setting description --------- Co-authored-by: GutoVeronezi --- .../consoleproxy/ConsoleAccessManager.java | 20 ++- .../java/com/cloud/vm/ConsoleSessionVO.java | 134 ++++++++++++++++++ .../com/cloud/vm/dao/ConsoleSessionDao.java | 36 +++++ .../cloud/vm/dao/ConsoleSessionDaoImpl.java | 69 +++++++++ ...spring-engine-schema-core-daos-context.xml | 1 + .../META-INF/db/schema-41600to41610.sql | 20 +++ .../com/cloud/consoleproxy/AgentHookBase.java | 5 +- .../ConsoleAccessManagerImpl.java | 103 ++++++++++++-- .../com/cloud/consoleproxy/ConsoleProxy.java | 5 +- .../consoleproxy/ConsoleProxyGCThread.java | 22 +-- .../consoleproxy/ConsoleProxyNoVncClient.java | 9 +- 11 files changed, 401 insertions(+), 23 deletions(-) create mode 100644 engine/schema/src/main/java/com/cloud/vm/ConsoleSessionVO.java create mode 100644 engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java create mode 100644 engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java diff --git a/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java b/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java index b1bd198309a..f19b5398dd9 100644 --- a/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java +++ b/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java @@ -18,12 +18,30 @@ package org.apache.cloudstack.consoleproxy; import com.cloud.utils.component.Manager; import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; -public interface ConsoleAccessManager extends Manager { +public interface ConsoleAccessManager extends Manager, Configurable { + + ConfigKey ConsoleSessionCleanupRetentionHours = new ConfigKey<>("Advanced", Integer.class, + "console.session.cleanup.retention.hours", + "240", + "Determines the hours to keep removed console session records before expunging them", + false, + ConfigKey.Scope.Global); + + ConfigKey ConsoleSessionCleanupInterval = new ConfigKey<>("Advanced", Integer.class, + "console.session.cleanup.interval", + "180", + "Determines the interval (in hours) to wait between the console session cleanup tasks", + false, + ConfigKey.Scope.Global); ConsoleEndpoint generateConsoleEndpoint(Long vmId, String extraSecurityToken, String clientAddress); boolean isSessionAllowed(String sessionUuid); void removeSessions(String[] sessionUuids); + + void acquireSession(String sessionUuid); } diff --git a/engine/schema/src/main/java/com/cloud/vm/ConsoleSessionVO.java b/engine/schema/src/main/java/com/cloud/vm/ConsoleSessionVO.java new file mode 100644 index 00000000000..4b476af463c --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/vm/ConsoleSessionVO.java @@ -0,0 +1,134 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.vm; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import java.util.Date; + +@Entity +@Table(name = "console_session") +public class ConsoleSessionVO { + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "id") + private long id; + + @Column(name = "uuid") + private String uuid; + + @Column(name = "created") + private Date created; + + @Column(name = "account_id") + private long accountId; + + @Column(name = "user_id") + private long userId; + + @Column(name = "instance_id") + private long instanceId; + + @Column(name = "host_id") + private long hostId; + + @Column(name = "acquired") + private boolean acquired; + + @Column(name = "removed") + private Date removed; + + public long getId() { + return id; + } + + public void setId(long id) { + this.id = id; + } + + public String getUuid() { + return uuid; + } + + public void setUuid(String uuid) { + this.uuid = uuid; + } + + public Date getCreated() { + return created; + } + + public void setCreated(Date created) { + this.created = created; + } + + public long getAccountId() { + return accountId; + } + + public void setAccountId(long accountId) { + this.accountId = accountId; + } + + public long getUserId() { + return userId; + } + + public void setUserId(long userId) { + this.userId = userId; + } + + public long getInstanceId() { + return instanceId; + } + + public void setInstanceId(long instanceId) { + this.instanceId = instanceId; + } + + public long getHostId() { + return hostId; + } + + public void setHostId(long hostId) { + this.hostId = hostId; + } + + public Date getRemoved() { + return removed; + } + + public void setRemoved(Date removed) { + this.removed = removed; + } + + public boolean isAcquired() { + return acquired; + } + + public void setAcquired(boolean acquired) { + this.acquired = acquired; + } +} diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java new file mode 100644 index 00000000000..71b1aed1938 --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java @@ -0,0 +1,36 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.vm.dao; + +import com.cloud.vm.ConsoleSessionVO; +import com.cloud.utils.db.GenericDao; + +import java.util.Date; + +public interface ConsoleSessionDao extends GenericDao { + + void removeSession(String sessionUuid); + + boolean isSessionAllowed(String sessionUuid); + + int expungeSessionsOlderThanDate(Date date); + + void acquireSession(String sessionUuid); +} diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java new file mode 100644 index 00000000000..f2f4703a2a2 --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java @@ -0,0 +1,69 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.vm.dao; + +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; +import com.cloud.vm.ConsoleSessionVO; +import com.cloud.utils.db.GenericDaoBase; + +import java.util.Date; + +public class ConsoleSessionDaoImpl extends GenericDaoBase implements ConsoleSessionDao { + + private final SearchBuilder searchByRemovedDate; + + public ConsoleSessionDaoImpl() { + searchByRemovedDate = createSearchBuilder(); + searchByRemovedDate.and("removedNotNull", searchByRemovedDate.entity().getRemoved(), SearchCriteria.Op.NNULL); + searchByRemovedDate.and("removed", searchByRemovedDate.entity().getRemoved(), SearchCriteria.Op.LTEQ); + } + + @Override + public void removeSession(String sessionUuid) { + ConsoleSessionVO session = findByUuid(sessionUuid); + remove(session.getId()); + } + + @Override + public boolean isSessionAllowed(String sessionUuid) { + ConsoleSessionVO consoleSessionVO = findByUuid(sessionUuid); + if (consoleSessionVO == null) { + return false; + } + return !consoleSessionVO.isAcquired(); + } + + @Override + public int expungeSessionsOlderThanDate(Date date) { + SearchCriteria searchCriteria = searchByRemovedDate.create(); + searchCriteria.setParameters("removed", date); + return expunge(searchCriteria); + } + + @Override + public void acquireSession(String sessionUuid) { + ConsoleSessionVO consoleSessionVO = findByUuid(sessionUuid); + consoleSessionVO.setAcquired(true); + update(consoleSessionVO.getId(), consoleSessionVO); + } + + +} diff --git a/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml b/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml index 860dcbac627..9ac23115695 100644 --- a/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml +++ b/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml @@ -49,6 +49,7 @@ + diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41600to41610.sql b/engine/schema/src/main/resources/META-INF/db/schema-41600to41610.sql index 56a0d3627cb..31ce3eae6aa 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41600to41610.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41600to41610.sql @@ -834,3 +834,23 @@ from `cloud`.`async_job` ON async_job.instance_id = vm_instance.id and async_job.instance_type = 'DomainRouter' and async_job.job_status = 0; + + +--- Create table for handling console sessions #7094 + +CREATE TABLE IF NOT EXISTS `cloud`.`console_session` ( + `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT PRIMARY KEY, + `uuid` varchar(40) NOT NULL COMMENT 'UUID generated for the session', + `created` datetime NOT NULL COMMENT 'When the session was created', + `account_id` bigint(20) unsigned NOT NULL COMMENT 'Account who generated the session', + `user_id` bigint(20) unsigned NOT NULL COMMENT 'User who generated the session', + `instance_id` bigint(20) unsigned NOT NULL COMMENT 'VM for which the session was generated', + `host_id` bigint(20) unsigned NOT NULL COMMENT 'Host where the VM was when the session was generated', + `acquired` int(1) NOT NULL DEFAULT 0 COMMENT 'True if the session was already used', + `removed` datetime COMMENT 'When the session was removed/used', + CONSTRAINT `fk_consolesession__account_id` FOREIGN KEY(`account_id`) REFERENCES `cloud`.`account` (`id`), + CONSTRAINT `fk_consolesession__user_id` FOREIGN KEY(`user_id`) REFERENCES `cloud`.`user`(`id`), + CONSTRAINT `fk_consolesession__instance_id` FOREIGN KEY(`instance_id`) REFERENCES `cloud`.`vm_instance`(`id`), + CONSTRAINT `fk_consolesession__host_id` FOREIGN KEY(`host_id`) REFERENCES `cloud`.`host`(`id`), + CONSTRAINT `uc_consolesession__uuid` UNIQUE (`uuid`) +); diff --git a/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java b/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java index 619825ecf43..efc5a1b5d84 100644 --- a/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java +++ b/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java @@ -106,10 +106,13 @@ public abstract class AgentHookBase implements AgentHook { } if (!consoleAccessManager.isSessionAllowed(sessionUuid)) { - s_logger.error("Invalid session, only one session allowed per token"); + s_logger.error(String.format("Session [%s] has been already used or does not exist.", sessionUuid)); return new ConsoleAccessAuthenticationAnswer(cmd, false); } + s_logger.debug(String.format("Acquiring session [%s] as it was just used.", sessionUuid)); + consoleAccessManager.acquireSession(sessionUuid); + if (!ticket.equals(ticketInUrl)) { Date now = new Date(); // considering of minute round-up diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index 15632b85f9b..c7ed967da3e 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -37,21 +37,29 @@ import com.cloud.uservm.UserVm; import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.utils.component.ManagerBase; +import com.cloud.utils.concurrency.NamedThreadFactory; import com.cloud.utils.db.EntityManager; +import com.cloud.utils.db.GlobalLock; import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.ConsoleSessionVO; import com.cloud.vm.UserVmDetailVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.VmDetailConstants; +import com.cloud.vm.dao.ConsoleSessionDao; import com.cloud.vm.dao.UserVmDetailsDao; import com.google.gson.Gson; import com.google.gson.GsonBuilder; import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.security.keys.KeysManager; +import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.commons.codec.binary.Base64; +import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; +import org.joda.time.DateTime; import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; @@ -60,11 +68,12 @@ import javax.naming.ConfigurationException; import java.util.Arrays; import java.util.Date; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.UUID; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAccessManager { @@ -84,6 +93,10 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce private AgentManager agentManager; @Inject private ConsoleProxyManager consoleProxyManager; + @Inject + private ConsoleSessionDao consoleSessionDao; + + private ScheduledExecutorService executorService = null; private static KeysManager secretKeysManager; private final Gson gson = new GsonBuilder().create(); @@ -94,15 +107,66 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce VirtualMachine.State.Stopped, VirtualMachine.State.Error, VirtualMachine.State.Destroyed ); - private static Set allowedSessions; - @Override public boolean configure(String name, Map params) throws ConfigurationException { ConsoleAccessManagerImpl.secretKeysManager = keysManager; - ConsoleAccessManagerImpl.allowedSessions = new HashSet<>(); + executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger")); return super.configure(name, params); } + @Override + public boolean start() { + int consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value(); + if (consoleCleanupInterval > 0) { + s_logger.info(String.format("The ConsoleSessionCleanupTask will run every %s hours", consoleCleanupInterval)); + executorService.scheduleWithFixedDelay(new ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval, TimeUnit.HOURS); + } + return true; + } + + @Override + public String getConfigComponentName() { + return ConsoleAccessManager.class.getName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[] { + ConsoleAccessManager.ConsoleSessionCleanupInterval, + ConsoleAccessManager.ConsoleSessionCleanupRetentionHours + }; + } + + public class ConsoleSessionCleanupTask extends ManagedContextRunnable { + @Override + protected void runInContext() { + final GlobalLock gcLock = GlobalLock.getInternLock("ConsoleSession.Cleanup.Lock"); + try { + if (gcLock.lock(3)) { + try { + reallyRun(); + } finally { + gcLock.unlock(); + } + } + } finally { + gcLock.releaseRef(); + } + } + + private void reallyRun() { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Starting ConsoleSessionCleanupTask..."); + } + Integer retentionHours = ConsoleAccessManager.ConsoleSessionCleanupRetentionHours.value(); + Date dateBefore = DateTime.now().minusHours(retentionHours).toDate(); + int sessionsExpunged = consoleSessionDao.expungeSessionsOlderThanDate(dateBefore); + if (sessionsExpunged > 0 && s_logger.isDebugEnabled()) { + s_logger.info(String.format("Expunged %s removed console session records", sessionsExpunged)); + } + } + } + @Override public ConsoleEndpoint generateConsoleEndpoint(Long vmId, String extraSecurityToken, String clientAddress) { try { @@ -144,16 +208,27 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce @Override public boolean isSessionAllowed(String sessionUuid) { - return allowedSessions.contains(sessionUuid); + return consoleSessionDao.isSessionAllowed(sessionUuid); } @Override public void removeSessions(String[] sessionUuids) { - for (String r : sessionUuids) { - allowedSessions.remove(r); + if (ArrayUtils.isNotEmpty(sessionUuids)) { + for (String sessionUuid : sessionUuids) { + removeSession(sessionUuid); + } } } + protected void removeSession(String sessionUuid) { + consoleSessionDao.removeSession(sessionUuid); + } + + @Override + public void acquireSession(String sessionUuid) { + consoleSessionDao.acquireSession(sessionUuid); + } + protected boolean checkSessionPermission(VirtualMachine vm, Account account) { if (accountManager.isRootAdmin(account.getId())) { return true; @@ -341,7 +416,7 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce s_logger.debug("Compose console url: " + sb); } s_logger.debug("Adding allowed session: " + sessionUuid); - allowedSessions.add(sessionUuid); + persistConsoleSession(sessionUuid, vm.getId(), hostVo.getId()); managementServer.setConsoleAccessForVm(vm.getId(), sessionUuid); String url = sb.toString().startsWith("https") ? sb.toString() : "http:" + sb; @@ -356,6 +431,16 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce return consoleEndpoint; } + protected void persistConsoleSession(String sessionUuid, long instanceId, long hostId) { + ConsoleSessionVO consoleSessionVo = new ConsoleSessionVO(); + consoleSessionVo.setUuid(sessionUuid); + consoleSessionVo.setAccountId(CallContext.current().getCallingAccountId()); + consoleSessionVo.setUserId(CallContext.current().getCallingUserId()); + consoleSessionVo.setInstanceId(instanceId); + consoleSessionVo.setHostId(hostId); + consoleSessionDao.persist(consoleSessionVo); + } + public static Ternary parseHostInfo(String hostInfo) { String host = null; String tunnelUrl = null; diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java index 9ad9e513fc5..bb12606dc38 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java @@ -31,6 +31,7 @@ import java.util.Hashtable; import java.util.Map; import java.util.Properties; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; import com.cloud.utils.StringUtils; @@ -67,6 +68,7 @@ public class ConsoleProxy { public static Method ensureRouteMethod; static Hashtable connectionMap = new Hashtable(); + static Set removedSessionsSet = ConcurrentHashMap.newKeySet(); static int httpListenPort = 80; static int httpCmdListenPort = 8001; static int reconnectMaxRetry = 5; @@ -366,7 +368,7 @@ public class ConsoleProxy { s_logger.info("HTTP command port is disabled"); } - ConsoleProxyGCThread cthread = new ConsoleProxyGCThread(connectionMap); + ConsoleProxyGCThread cthread = new ConsoleProxyGCThread(connectionMap, removedSessionsSet); cthread.setName("Console Proxy GC Thread"); cthread.start(); } @@ -534,6 +536,7 @@ public class ConsoleProxy { for (Map.Entry entry : connectionMap.entrySet()) { if (entry.getValue() == viewer) { connectionMap.remove(entry.getKey()); + removedSessionsSet.add(viewer.getSessionUuid()); return; } } diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java index de9577408c9..d7aaafcea0d 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java @@ -20,7 +20,7 @@ import java.io.File; import java.util.ArrayList; import java.util.Enumeration; import java.util.Hashtable; -import java.util.List; +import java.util.Set; import org.apache.log4j.Logger; @@ -36,10 +36,12 @@ public class ConsoleProxyGCThread extends Thread { private final static int MAX_SESSION_IDLE_SECONDS = 180; private final Hashtable connMap; + private final Set removedSessionsSet; private long lastLogScan = 0; - public ConsoleProxyGCThread(Hashtable connMap) { + public ConsoleProxyGCThread(Hashtable connMap, Set removedSet) { this.connMap = connMap; + this.removedSessionsSet = removedSet; } private void cleanupLogging() { @@ -69,15 +71,14 @@ public class ConsoleProxyGCThread extends Thread { boolean bReportLoad = false; long lastReportTick = System.currentTimeMillis(); - List removedSessions = new ArrayList<>(); while (true) { cleanupLogging(); bReportLoad = false; - removedSessions.clear(); - if (s_logger.isDebugEnabled()) - s_logger.debug("connMap=" + connMap); + if (s_logger.isDebugEnabled()) { + s_logger.debug(String.format("connMap=%s, removedSessions=%s", connMap, removedSessionsSet)); + } Enumeration e = connMap.keys(); while (e.hasMoreElements()) { String key; @@ -94,7 +95,6 @@ public class ConsoleProxyGCThread extends Thread { } synchronized (connMap) { - removedSessions.add(client.getSessionUuid()); connMap.remove(key); bReportLoad = true; } @@ -107,13 +107,17 @@ public class ConsoleProxyGCThread extends Thread { if (bReportLoad || System.currentTimeMillis() - lastReportTick > 5000) { // report load changes ConsoleProxyClientStatsCollector collector = new ConsoleProxyClientStatsCollector(connMap); - collector.setRemovedSessions(removedSessions); + collector.setRemovedSessions(new ArrayList<>(removedSessionsSet)); String loadInfo = collector.getStatsReport(); ConsoleProxy.reportLoadInfo(loadInfo); lastReportTick = System.currentTimeMillis(); + synchronized (removedSessionsSet) { + removedSessionsSet.clear(); + } - if (s_logger.isDebugEnabled()) + if (s_logger.isDebugEnabled()) { s_logger.debug("Report load change : " + loadInfo); + } } try { diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java index ff777976184..36ba2a7b26b 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java @@ -19,6 +19,7 @@ package com.cloud.consoleproxy; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.eclipse.jetty.websocket.api.Session; +import org.eclipse.jetty.websocket.api.WebSocketException; import org.eclipse.jetty.websocket.api.extensions.Frame; import java.awt.Image; @@ -129,8 +130,12 @@ public class ConsoleProxyNoVncClient implements ConsoleProxyClient { break; } if (readBytes > 0) { - session.getRemote().sendBytes(ByteBuffer.wrap(b, 0, readBytes)); - updateFrontEndActivityTime(); + try { + session.getRemote().sendBytes(ByteBuffer.wrap(b, 0, readBytes)); + updateFrontEndActivityTime(); + } catch (WebSocketException e) { + connectionAlive = false; + } } } }