This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 38aff8f0ee8a5cfa2b383bd56a053db6d59d50c3
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed May 5 15:14:30 2021 +0100
Complete fix for BZ 65262. Encoders and decoders now use InstanceManager
https://bz.apache.org/bugzilla/show_bug.cgi?id=65262
---
.../apache/tomcat/websocket/LocalStrings.properties | 1 +
.../tomcat/websocket/WsRemoteEndpointImplBase.java | 20 ++++++++++++++++++--
.../tomcat/websocket/server/WsServerContainer.java | 20 +++++++++++++-------
webapps/docs/changelog.xml | 6 +++---
4 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/LocalStrings.properties
b/java/org/apache/tomcat/websocket/LocalStrings.properties
index ae54da0..fbd7bfd 100644
--- a/java/org/apache/tomcat/websocket/LocalStrings.properties
+++ b/java/org/apache/tomcat/websocket/LocalStrings.properties
@@ -92,6 +92,7 @@ wsRemoteEndpoint.closed=Message will not be sent because the
WebSocket session h
wsRemoteEndpoint.closedDuringMessage=The remainder of the message will not be
sent because the WebSocket session has been closed
wsRemoteEndpoint.closedOutputStream=This method may not be called as the
OutputStream has been closed
wsRemoteEndpoint.closedWriter=This method may not be called as the Writer has
been closed
+wsRemoteEndpoint.encoderDestoryFailed=Failed to destroy the encoder of type
[{0}]
wsRemoteEndpoint.flushOnCloseFailed=Batched messages still enabled after
session has been closed. Unable to flush remaining batched message.
wsRemoteEndpoint.invalidEncoder=The specified encoder of type [{0}] could not
be instantiated
wsRemoteEndpoint.noEncoder=No encoder specified for object of class [{0}]
diff --git a/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
b/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
index 18c723a..11f6fd4 100644
--- a/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
+++ b/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
@@ -19,6 +19,7 @@ package org.apache.tomcat.websocket;
import java.io.IOException;
import java.io.OutputStream;
import java.io.Writer;
+import java.lang.reflect.InvocationTargetException;
import java.net.SocketTimeoutException;
import java.nio.ByteBuffer;
import java.nio.CharBuffer;
@@ -33,6 +34,7 @@ import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
+import javax.naming.NamingException;
import javax.websocket.CloseReason;
import javax.websocket.CloseReason.CloseCodes;
import javax.websocket.DeploymentException;
@@ -45,6 +47,7 @@ import javax.websocket.SendResult;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.InstanceManager;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.buf.Utf8Encoder;
import org.apache.tomcat.util.res.StringManager;
@@ -704,10 +707,15 @@ public abstract class WsRemoteEndpointImplBase implements
RemoteEndpoint {
for (Class<? extends Encoder> encoderClazz :
endpointConfig.getEncoders()) {
Encoder instance;
+ InstanceManager instanceManager = wsSession.getInstanceManager();
try {
- instance = encoderClazz.getConstructor().newInstance();
+ if (instanceManager == null) {
+ instance = encoderClazz.getConstructor().newInstance();
+ } else {
+ instance = (Encoder)
instanceManager.newInstance(encoderClazz);
+ }
instance.init(endpointConfig);
- } catch (ReflectiveOperationException e) {
+ } catch (ReflectiveOperationException | NamingException e) {
throw new DeploymentException(
sm.getString("wsRemoteEndpoint.invalidEncoder",
encoderClazz.getName()), e);
@@ -730,8 +738,16 @@ public abstract class WsRemoteEndpointImplBase implements
RemoteEndpoint {
public final void close() {
+ InstanceManager instanceManager = wsSession.getInstanceManager();
for (EncoderEntry entry : encoderEntries) {
entry.getEncoder().destroy();
+ if (instanceManager != null) {
+ try {
+ instanceManager.destroyInstance(entry);
+ } catch (IllegalAccessException | InvocationTargetException e)
{
+
log.warn(sm.getString("wsRemoteEndpoint.encoderDestoryFailed",
encoder.getClass()), e);
+ }
+ }
}
// The transformation handles both input and output. It only needs to
be
// closed once so it is closed here on the output side.
diff --git a/java/org/apache/tomcat/websocket/server/WsServerContainer.java
b/java/org/apache/tomcat/websocket/server/WsServerContainer.java
index 15df2d7..3a8ee52 100644
--- a/java/org/apache/tomcat/websocket/server/WsServerContainer.java
+++ b/java/org/apache/tomcat/websocket/server/WsServerContainer.java
@@ -26,6 +26,7 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ConcurrentSkipListMap;
+import javax.naming.NamingException;
import javax.servlet.DispatcherType;
import javax.servlet.FilterRegistration;
import javax.servlet.ServletContext;
@@ -250,7 +251,7 @@ public class WsServerContainer extends WsWebSocketContainer
String path = annotation.value();
// Validate encoders
- validateEncoders(annotation.encoders());
+ validateEncoders(annotation.encoders(),
getInstanceManager(Thread.currentThread().getContextClassLoader()));
// ServerEndpointConfig
Class<? extends Configurator> configuratorClazz =
@@ -465,17 +466,22 @@ public class WsServerContainer extends
WsWebSocketContainer
}
- private static void validateEncoders(Class<? extends Encoder>[] encoders)
+ private static void validateEncoders(Class<? extends Encoder>[] encoders,
InstanceManager instanceManager)
throws DeploymentException {
for (Class<? extends Encoder> encoder : encoders) {
- // Need to instantiate decoder to ensure it is valid and that
- // deployment can be failed if it is not
- @SuppressWarnings("unused")
+ // Need to instantiate encoder to ensure it is valid and that
+ // deployment can be failed if it is not. The encoder is then
+ // discarded immediately.
Encoder instance;
try {
- encoder.getConstructor().newInstance();
- } catch(ReflectiveOperationException e) {
+ if (instanceManager == null) {
+ instance = encoder.getConstructor().newInstance();
+ } else {
+ instance = (Encoder) instanceManager.newInstance(encoder);
+ instanceManager.destroyInstance(instance);
+ }
+ } catch(ReflectiveOperationException | NamingException e) {
throw new DeploymentException(sm.getString(
"serverContainer.encoderFail", encoder.getName()), e);
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index fda0589..08ff055 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -180,9 +180,9 @@
simplify the code. (markt)
</scode>
<fix>
- <bug>65262</bug>: Refactor the creation of WebSocket end point
instances
- to be more IoC friendly. Instances are now created via the
- <code>InstanceManager</code> where possible. (markt)
+ <bug>65262</bug>: Refactor the creation of WebSocket end point, decoder
+ and encoder instances to be more IoC friendly. Instances are now
created
+ via the <code>InstanceManager</code> where possible. (markt)
</fix>
</changelog>
</subsection>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
