Fix for leaking WsSession in server container

Red5 · Oct 21, 2024 · 38d184b · 38d184b
1 parent 46b3129
commit 38d184b
Show file tree

Hide file tree

Showing 12 changed files with 40 additions and 42 deletions.
diff --git a/client/pom.xml b/client/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <groupId>org.red5</groupId>
         <artifactId>red5-parent</artifactId>
-        <version>1.3.36</version>
+        <version>1.3.37</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
     <artifactId>red5-client</artifactId>

diff --git a/client/src/main/java/org/red5/client/Red5Client.java b/client/src/main/java/org/red5/client/Red5Client.java
@@ -18,7 +18,7 @@ public final class Red5Client {
     /**
      * Current server version with revision
      */
-    public static final String VERSION = "Red5 Client 1.3.36";
+    public static final String VERSION = "Red5 Client 1.3.37";
 
     /**
      * Create a new Red5Client object using the connection local to the current thread A bit of magic that lets you access the red5 scope

diff --git a/common/pom.xml b/common/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <groupId>org.red5</groupId>
         <artifactId>red5-parent</artifactId>
-        <version>1.3.36</version>
+        <version>1.3.37</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
     <artifactId>red5-server-common</artifactId>
@@ -105,7 +105,7 @@
         <dependency>
             <groupId>net.engio</groupId>
             <artifactId>mbassador</artifactId>
-            <version>1.3.36</version>
+            <version>1.3.37</version>
         </dependency> -->
         <dependency>
             <groupId>junit</groupId>

diff --git a/common/src/main/java/org/red5/server/api/Red5.java b/common/src/main/java/org/red5/server/api/Red5.java
@@ -57,7 +57,7 @@ public final class Red5 {
     /**
      * Server version with revision
      */
-    public static final String VERSION = "Red5 Server 1.3.36";
+    public static final String VERSION = "Red5 Server 1.3.37";
 
     /**
      * Server version for fmsVer requests

diff --git a/io/pom.xml b/io/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <groupId>org.red5</groupId>
         <artifactId>red5-parent</artifactId>
-        <version>1.3.36</version>
+        <version>1.3.37</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
     <artifactId>red5-io</artifactId>

diff --git a/io/src/main/java/org/red5/io/utils/TlsUtils.java b/io/src/main/java/org/red5/io/utils/TlsUtils.java
@@ -22,9 +22,8 @@
 @SuppressWarnings({ "rawtypes", "unchecked" })
 public class TlsUtils {
 
-    private static byte[] DOWNGRADE_TLS11 = Hex.decodeStrict("444F574E47524400");
-
-    private static byte[] DOWNGRADE_TLS12 = Hex.decodeStrict("444F574E47524401");
+    @SuppressWarnings("unused")
+    private static byte[] DOWNGRADE_TLS11 = Hex.decodeStrict("444F574E47524400"), DOWNGRADE_TLS12 = Hex.decodeStrict("444F574E47524401");
 
     public static final byte[] EMPTY_BYTES = new byte[0];
 
@@ -608,15 +607,16 @@ public static int[] readUint16Array(int count, InputStream input) throws IOExcep
     }
 
     public static ASN1Primitive readASN1Object(byte[] encoding) throws IOException {
-        ASN1InputStream asn1 = new ASN1InputStream(encoding);
-        ASN1Primitive result = asn1.readObject();
-        if (null == result) {
-            throw new IOException("AlertDescription.decode_error");
-        }
-        if (null != asn1.readObject()) {
-            throw new IOException("AlertDescription.decode_error");
+        try (ASN1InputStream asn1 = new ASN1InputStream(encoding)) {
+            ASN1Primitive result = asn1.readObject();
+            if (null == result) {
+                throw new IOException("AlertDescription.decode_error");
+            }
+            if (null != asn1.readObject()) {
+                throw new IOException("AlertDescription.decode_error");
+            }
+            return result;
         }
-        return result;
     }
 
     /** @deprecated Will be removed. Use readASN1Object in combination with requireDEREncoding instead */
@@ -739,6 +739,7 @@ static byte[] concat(byte[] a, byte[] b) {
         return c;
     }
 
+    @SuppressWarnings("unused")
     private static byte[] getCertificateVerifyHeader(String contextString) {
         int count = contextString.length();
         byte[] header = new byte[64 + count + 1];

diff --git a/pom.xml b/pom.xml
@@ -24,7 +24,7 @@
     <name>Red5</name>
     <description>The Red5 server</description>
     <groupId>org.red5</groupId>
-    <version>1.3.36</version>
+    <version>1.3.37</version>
     <url>https://github.com/Red5/red5-server</url>
     <inceptionYear>2005</inceptionYear>
     <organization>

diff --git a/server/pom.xml b/server/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <groupId>org.red5</groupId>
         <artifactId>red5-parent</artifactId>
-        <version>1.3.36</version>
+        <version>1.3.37</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
     <artifactId>red5-server</artifactId>

diff --git a/server/src/main/java/org/red5/net/websocket/WebSocketConnection.java b/server/src/main/java/org/red5/net/websocket/WebSocketConnection.java
@@ -63,7 +63,7 @@ public class WebSocketConnection extends AttributeStore implements Comparable<We
     private AtomicBoolean connected = new AtomicBoolean(false);
 
     // associated websocket session
-    private WeakReference<WsSession> wsSession;
+    private final WsSession wsSession;
 
     private WeakReference<WebSocketScope> scope;
 
@@ -114,9 +114,9 @@ public WebSocketConnection(WebSocketScope scope, Session session) {
             log.debug("path: {}", path);
         }
         // cast ws session
-        this.wsSession = new WeakReference<>((WsSession) session);
+        this.wsSession = (WsSession) session;
         if (isDebug) {
-            log.debug("ws session: {}", wsSession.get());
+            log.debug("ws session: {}", wsSession);
         }
         // the websocket session id will be used for hash code comparison, its the only usable value currently
         wsSessionId = session.getId();
@@ -186,7 +186,7 @@ public void send(String data) throws UnsupportedEncodingException, IOException {
         }
         // process the incoming string
         if (StringUtils.isNotBlank(data)) {
-            final WsSession session = wsSession.get();
+            final WsSession session = wsSession;
             // attempt send only if the session is not closed
             if (session != null && !session.isClosed()) {
                 try {
@@ -236,7 +236,7 @@ public void send(byte[] buf) throws IOException {
         if (isDebug) {
             log.debug("send binary: {}", Arrays.toString(buf));
         }
-        WsSession session = wsSession.get();
+        WsSession session = wsSession;
         if (session != null && session.isOpen()) {
             try {
                 // send the bytes
@@ -281,7 +281,7 @@ public void sendPing(byte[] buf) throws IllegalArgumentException, IOException {
         if (isTrace) {
             log.trace("send ping: {}", buf);
         }
-        WsSession session = wsSession.get();
+        WsSession session = wsSession;
         if (session != null && session.isOpen()) {
             synchronized (wsSessionId) {
                 // send the bytes
@@ -305,7 +305,7 @@ public void sendPong(byte[] buf) throws IllegalArgumentException, IOException {
         if (isTrace) {
             log.trace("send pong: {}", buf);
         }
-        WsSession session = wsSession.get();
+        WsSession session = wsSession;
         if (session != null && session.isOpen()) {
             synchronized (wsSessionId) {
                 // send the bytes
@@ -324,15 +324,11 @@ public void sendPong(byte[] buf) throws IllegalArgumentException, IOException {
     public void close() {
         if (connected.compareAndSet(true, false)) {
             log.debug("close: {}", wsSessionId);
-            WsSession session = wsSession != null ? wsSession.get() : null;
-            // session has to be open, or user props cannot be retrieved
-            if (session != null && session.isOpen()) {
-                // trying to close the session nicely
-                try {
-                    session.close();
-                } catch (Exception e) {
-                    log.debug("Exception closing session", e);
-                }
+            // trying to close the session nicely
+            try {
+                wsSession.close();
+            } catch (Exception e) {
+                log.debug("Exception closing session", e);
             }
             // clean up our props
             attributes.clear();
@@ -351,7 +347,6 @@ public void close() {
                 // disconnect from scope
                 scope.get().removeConnection(this);
                 // clear weak refs
-                wsSession.clear();
                 scope.clear();
             }
         }
@@ -369,7 +364,7 @@ public void timeoutAsync(long now) {
         // XXX(paul) only logging here as we should more than likely rely upon the container checking expiration
         log.trace("timeoutAsync: {} on session id: {} read: {} written: {}", now, wsSessionId, readBytes, writtenBytes);
         /*
-        WsSession session = wsSession.get();
+        WsSession session = wsSession;
         Map<String, Object> props = session.getUserProperties();
         log.debug("Session properties: {}", props);
         long maxIdleTimeout = session.getMaxIdleTimeout();
@@ -453,7 +448,7 @@ public void setOrigin(String origin) {
      * @return true if secure and false if unsecure or unconnected
      */
     public boolean isSecure() {
-        Optional<WsSession> opt = Optional.ofNullable(wsSession.get());
+        Optional<WsSession> opt = Optional.ofNullable(wsSession);
         if (opt.isPresent()) {
             return (opt.get().isOpen() ? opt.get().isSecure() : false);
         }
@@ -672,12 +667,12 @@ public Object getUserProperty(String key) {
 
     public void setWsSessionTimeout(long idleTimeout) {
         if (wsSession != null) {
-            wsSession.get().setMaxIdleTimeout(idleTimeout);
+            wsSession.setMaxIdleTimeout(idleTimeout);
         }
     }
 
     public WsSession getWsSession() {
-        return wsSession != null ? wsSession.get() : null;
+        return wsSession != null ? wsSession : null;
     }
 
     public long getReadBytes() {

diff --git a/server/src/main/java/org/red5/net/websocket/WebSocketScopeManager.java b/server/src/main/java/org/red5/net/websocket/WebSocketScopeManager.java
@@ -181,6 +181,8 @@ public boolean addWebSocketScope(WebSocketScope webSocketScope) {
                                             log.debug("Removing unconnected connection: {} during ping loop", wsConn.getSessionId());
                                             // if the connection isn't connected, remove them
                                             wsScope.removeConnection(wsConn);
+                                            // if connection is not connected, close it (ensure closed / removed)
+                                            wsConn.close();
                                         }
                                     } catch (Exception e) {
                                         log.warn("Exception in WS pinger", e);

diff --git a/service/pom.xml b/service/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <groupId>org.red5</groupId>
         <artifactId>red5-parent</artifactId>
-        <version>1.3.36</version>
+        <version>1.3.37</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
     <artifactId>red5-service</artifactId>

diff --git a/servlet/pom.xml b/servlet/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <groupId>org.red5</groupId>
         <artifactId>red5-parent</artifactId>
-        <version>1.3.36</version>
+        <version>1.3.37</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
     <artifactId>red5-servlet</artifactId>