This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit c48f647d951f20d5b51a5ad3f3dbd07be05c3e3e
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Apr 28 12:47:31 2021 +0100
Create/destroy Decoders via the InstanceManager (BZ 65262)
---
.../apache/tomcat/websocket/PojoClassHolder.java | 2 +-
java/org/apache/tomcat/websocket/PojoHolder.java | 2 +-
java/org/apache/tomcat/websocket/Util.java | 80 ++++++++++++++++------
.../tomcat/websocket/pojo/LocalStrings.properties | 2 +
.../tomcat/websocket/pojo/PojoEndpointClient.java | 9 +++
.../pojo/PojoMessageHandlerWholeBase.java | 29 ++++++++
.../pojo/PojoMessageHandlerWholeBinary.java | 12 ++--
.../pojo/PojoMessageHandlerWholeText.java | 12 ++--
.../tomcat/websocket/pojo/PojoMethodMapping.java | 37 ++++++++--
.../tomcat/websocket/server/WsServerContainer.java | 2 +-
.../tomcat/websocket/TesterWsClientAutobahn.java | 2 +-
11 files changed, 148 insertions(+), 41 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/PojoClassHolder.java
b/java/org/apache/tomcat/websocket/PojoClassHolder.java
index e2c1da8..61e338b 100644
--- a/java/org/apache/tomcat/websocket/PojoClassHolder.java
+++ b/java/org/apache/tomcat/websocket/PojoClassHolder.java
@@ -54,7 +54,7 @@ public class PojoClassHolder implements ClientEndpointHolder {
} else {
pojo = instanceManager.newInstance(pojoClazz);
}
- return new PojoEndpointClient(pojo,
clientEndpointConfig.getDecoders());
+ return new PojoEndpointClient(pojo,
clientEndpointConfig.getDecoders(), instanceManager);
} catch (ReflectiveOperationException | SecurityException |
NamingException e) {
throw new
DeploymentException(sm.getString("clientEndpointHolder.instanceCreationFailed"),
e);
}
diff --git a/java/org/apache/tomcat/websocket/PojoHolder.java
b/java/org/apache/tomcat/websocket/PojoHolder.java
index 7195944..8ff3f42 100644
--- a/java/org/apache/tomcat/websocket/PojoHolder.java
+++ b/java/org/apache/tomcat/websocket/PojoHolder.java
@@ -55,6 +55,6 @@ public class PojoHolder implements ClientEndpointHolder {
throw new
DeploymentException(sm.getString("clientEndpointHolder.instanceRegistrationFailed"),
e);
}
}
- return new PojoEndpointClient(pojo,
clientEndpointConfig.getDecoders());
+ return new PojoEndpointClient(pojo,
clientEndpointConfig.getDecoders(), instanceManager);
}
}
diff --git a/java/org/apache/tomcat/websocket/Util.java
b/java/org/apache/tomcat/websocket/Util.java
index ac8ea2d..6095f54 100644
--- a/java/org/apache/tomcat/websocket/Util.java
+++ b/java/org/apache/tomcat/websocket/Util.java
@@ -33,6 +33,8 @@ import java.util.Queue;
import java.util.Set;
import java.util.concurrent.ConcurrentLinkedQueue;
+import javax.naming.NamingException;
+
import jakarta.websocket.CloseReason.CloseCode;
import jakarta.websocket.CloseReason.CloseCodes;
import jakarta.websocket.Decoder;
@@ -48,6 +50,7 @@ import jakarta.websocket.MessageHandler;
import jakarta.websocket.PongMessage;
import jakarta.websocket.Session;
+import org.apache.tomcat.InstanceManager;
import org.apache.tomcat.util.res.StringManager;
import org.apache.tomcat.websocket.pojo.PojoMessageHandlerPartialBinary;
import org.apache.tomcat.websocket.pojo.PojoMessageHandlerWholeBinary;
@@ -330,26 +333,59 @@ public class Util {
}
- public static List<DecoderEntry> getDecoders(
- List<Class<? extends Decoder>> decoderClazzes)
- throws DeploymentException{
+ /**
+ * Build the list of decoder entries from a set of decoder implementations.
+ *
+ * @param decoderClazzes Decoder implementation classes
+ *
+ * @return List of mappings from target type to associated decoder
+ *
+ * @throws DeploymentException If a provided decoder class is not valid
+ *
+ * @deprecated Will be removed in Tomcat 10.1.x.
+ * Use {@link Util#getDecoders(List, InstanceManager)}
+ */
+ @Deprecated
+ public static List<DecoderEntry> getDecoders(List<Class<? extends
Decoder>> decoderClazzes)
+ throws DeploymentException {
+ return getDecoders(decoderClazzes, null);
+ }
+
+
+ /**
+ * Build the list of decoder entries from a set of decoder implementations.
+ *
+ * @param decoderClazzes Decoder implementation classes
+ * @param instanceManager Instance manager to use to create Decoder
+ * instances
+ *
+ * @return List of mappings from target type to associated decoder
+ *
+ * @throws DeploymentException If a provided decoder class is not valid
+ */
+ public static List<DecoderEntry> getDecoders(List<Class<? extends
Decoder>> decoderClazzes,
+ InstanceManager instanceManager) throws DeploymentException{
List<DecoderEntry> result = new ArrayList<>();
if (decoderClazzes != null) {
for (Class<? extends Decoder> decoderClazz : decoderClazzes) {
// Need to instantiate decoder to ensure it is valid and that
// deployment can be failed if it is not
- @SuppressWarnings("unused")
Decoder instance;
try {
- instance = decoderClazz.getConstructor().newInstance();
- } catch (ReflectiveOperationException e) {
+ if (instanceManager == null) {
+ instance = decoderClazz.getConstructor().newInstance();
+ } else {
+ instance = (Decoder)
instanceManager.newInstance(decoderClazz);
+ // Don't need this instance, so destroy it
+ instanceManager.destroyInstance(instance);
+ }
+ } catch (ReflectiveOperationException |
IllegalArgumentException | SecurityException |
+ NamingException e) {
throw new DeploymentException(
- sm.getString("pojoMethodMapping.invalidDecoder",
- decoderClazz.getName()), e);
+ sm.getString("pojoMethodMapping.invalidDecoder",
decoderClazz.getName()), e);
}
- DecoderEntry entry = new DecoderEntry(
- Util.getDecoderType(decoderClazz), decoderClazz);
+ DecoderEntry entry = new
DecoderEntry(Util.getDecoderType(decoderClazz), decoderClazz);
result.add(entry);
}
}
@@ -388,8 +424,9 @@ public class Util {
boolean whole =
MessageHandler.Whole.class.isAssignableFrom(listener.getClass());
MessageHandlerResult result = new MessageHandlerResult(
whole ? new PojoMessageHandlerWholeBinary(listener,
- getOnMessageMethod(listener), session,
- endpointConfig, matchDecoders(target,
endpointConfig, true),
+ getOnMessageMethod(listener), session,
endpointConfig,
+ matchDecoders(target, endpointConfig, true,
+ ((WsSession)
session).getInstanceManager()),
new Object[1], 0, true, -1, false, -1) :
new PojoMessageHandlerPartialBinary(listener,
getOnMessagePartialMethod(listener),
session,
@@ -399,23 +436,24 @@ public class Util {
} else if (InputStream.class.isAssignableFrom(target)) {
MessageHandlerResult result = new MessageHandlerResult(
new PojoMessageHandlerWholeBinary(listener,
- getOnMessageMethod(listener), session,
- endpointConfig, matchDecoders(target,
endpointConfig, true),
+ getOnMessageMethod(listener), session,
endpointConfig,
+ matchDecoders(target, endpointConfig, true,
((WsSession) session).getInstanceManager()),
new Object[1], 0, true, -1, true, -1),
MessageHandlerResultType.BINARY);
results.add(result);
} else if (Reader.class.isAssignableFrom(target)) {
MessageHandlerResult result = new MessageHandlerResult(
new PojoMessageHandlerWholeText(listener,
- getOnMessageMethod(listener), session,
- endpointConfig, matchDecoders(target,
endpointConfig, false),
+ getOnMessageMethod(listener), session,
endpointConfig,
+ matchDecoders(target, endpointConfig, false,
((WsSession) session).getInstanceManager()),
new Object[1], 0, true, -1, -1),
MessageHandlerResultType.TEXT);
results.add(result);
} else {
// Handler needs wrapping and requires decoder to convert it to one
// of the types expected by the frame handling code
- DecoderMatch decoderMatch = matchDecoders(target, endpointConfig);
+ DecoderMatch decoderMatch = matchDecoders(target, endpointConfig,
+ ((WsSession) session).getInstanceManager());
Method m = getOnMessageMethod(listener);
if (decoderMatch.getBinaryDecoders().size() > 0) {
MessageHandlerResult result = new MessageHandlerResult(
@@ -446,8 +484,8 @@ public class Util {
}
private static List<Class<? extends Decoder>> matchDecoders(Class<?>
target,
- EndpointConfig endpointConfig, boolean binary) {
- DecoderMatch decoderMatch = matchDecoders(target, endpointConfig);
+ EndpointConfig endpointConfig, boolean binary, InstanceManager
instanceManager) {
+ DecoderMatch decoderMatch = matchDecoders(target, endpointConfig,
instanceManager);
if (binary) {
if (decoderMatch.getBinaryDecoders().size() > 0) {
return decoderMatch.getBinaryDecoders();
@@ -459,12 +497,12 @@ public class Util {
}
private static DecoderMatch matchDecoders(Class<?> target,
- EndpointConfig endpointConfig) {
+ EndpointConfig endpointConfig, InstanceManager instanceManager) {
DecoderMatch decoderMatch;
try {
List<Class<? extends Decoder>> decoders =
endpointConfig.getDecoders();
- List<DecoderEntry> decoderEntries = getDecoders(decoders);
+ List<DecoderEntry> decoderEntries = getDecoders(decoders,
instanceManager);
decoderMatch = new DecoderMatch(target, decoderEntries);
} catch (DeploymentException e) {
throw new IllegalArgumentException(e);
diff --git a/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties
b/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties
index 50b47c3..afefa8c 100644
--- a/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties
+++ b/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties
@@ -22,6 +22,8 @@ pojoEndpointBase.onOpenFail=Failed to call onOpen method of
POJO end point for P
pojoMessageHandlerWhole.decodeIoFail=IO error while decoding message
pojoMessageHandlerWhole.maxBufferSize=The maximum supported message size for
this implementation is Integer.MAX_VALUE
+pojoMessageHandlerWholeBase.decodeDestoryFailed=Failed to destroy the decoder
of type [{0}]
+
pojoMethodMapping.decodePathParamFail=Failed to decode path parameter value
[{0}] to expected type [{1}]
pojoMethodMapping.duplicateAnnotation=Duplicate annotations [{0}] present on
class [{1}]
pojoMethodMapping.duplicateLastParam=Multiple boolean (last) parameters
present on the method [{0}] of class [{1}] that was annotated with OnMessage
diff --git a/java/org/apache/tomcat/websocket/pojo/PojoEndpointClient.java
b/java/org/apache/tomcat/websocket/pojo/PojoEndpointClient.java
index 66033f3..2aebe00 100644
--- a/java/org/apache/tomcat/websocket/pojo/PojoEndpointClient.java
+++ b/java/org/apache/tomcat/websocket/pojo/PojoEndpointClient.java
@@ -24,6 +24,8 @@ import jakarta.websocket.DeploymentException;
import jakarta.websocket.EndpointConfig;
import jakarta.websocket.Session;
+import org.apache.tomcat.InstanceManager;
+
/**
* Wrapper class for instances of POJOs annotated with
* {@link jakarta.websocket.ClientEndpoint} so they appear as standard
@@ -31,6 +33,7 @@ import jakarta.websocket.Session;
*/
public class PojoEndpointClient extends PojoEndpointBase {
+ @Deprecated
public PojoEndpointClient(Object pojo,
List<Class<? extends Decoder>> decoders) throws
DeploymentException {
super(Collections.<String,String>emptyMap());
@@ -38,6 +41,12 @@ public class PojoEndpointClient extends PojoEndpointBase {
setMethodMapping(new PojoMethodMapping(pojo.getClass(), decoders,
null));
}
+ public PojoEndpointClient(Object pojo, List<Class<? extends Decoder>>
decoders, InstanceManager instanceManager) throws DeploymentException {
+ super(Collections.<String,String>emptyMap());
+ setPojo(pojo);
+ setMethodMapping(new PojoMethodMapping(pojo.getClass(), decoders,
null, instanceManager));
+ }
+
@Override
public void onOpen(Session session, EndpointConfig config) {
doOnOpen(session, config);
diff --git
a/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeBase.java
b/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeBase.java
index 90d7398..4fdb87c 100644
--- a/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeBase.java
+++ b/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeBase.java
@@ -21,11 +21,17 @@ import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;
+import javax.naming.NamingException;
+
import jakarta.websocket.DecodeException;
import jakarta.websocket.Decoder;
import jakarta.websocket.MessageHandler;
import jakarta.websocket.Session;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.InstanceManager;
+import org.apache.tomcat.util.res.StringManager;
import org.apache.tomcat.websocket.WsSession;
/**
@@ -37,6 +43,9 @@ import org.apache.tomcat.websocket.WsSession;
public abstract class PojoMessageHandlerWholeBase<T>
extends PojoMessageHandlerBase<T> implements MessageHandler.Whole<T> {
+ private final Log log =
LogFactory.getLog(PojoMessageHandlerWholeBase.class); // must not be static
+ private static final StringManager sm =
StringManager.getManager(PojoMessageHandlerWholeBase.class);
+
protected final List<Decoder> decoders = new ArrayList<>();
public PojoMessageHandlerWholeBase(Object pojo, Method method,
@@ -47,6 +56,17 @@ public abstract class PojoMessageHandlerWholeBase<T>
}
+ protected Decoder createDecoderInstance(Class<? extends Decoder> clazz)
+ throws ReflectiveOperationException, NamingException {
+ InstanceManager instanceManager = ((WsSession)
session).getInstanceManager();
+ if (instanceManager == null) {
+ return clazz.getConstructor().newInstance();
+ } else {
+ return (Decoder) instanceManager.newInstance(clazz);
+ }
+ }
+
+
@Override
public final void onMessage(T message) {
@@ -91,8 +111,17 @@ public abstract class PojoMessageHandlerWholeBase<T>
protected void onClose() {
+ InstanceManager instanceManager = ((WsSession)
session).getInstanceManager();
+
for (Decoder decoder : decoders) {
decoder.destroy();
+ if (instanceManager != null) {
+ try {
+ instanceManager.destroyInstance(decoder);
+ } catch (IllegalAccessException | InvocationTargetException e)
{
+
log.warn(sm.getString("pojoMessageHandlerWholeBase.decodeDestoryFailed",
decoder.getClass()), e);
+ }
+ }
}
}
diff --git
a/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeBinary.java
b/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeBinary.java
index 93feeeb..9a7699d 100644
--- a/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeBinary.java
+++ b/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeBinary.java
@@ -22,6 +22,8 @@ import java.lang.reflect.Method;
import java.nio.ByteBuffer;
import java.util.List;
+import javax.naming.NamingException;
+
import jakarta.websocket.DecodeException;
import jakarta.websocket.Decoder;
import jakarta.websocket.Decoder.Binary;
@@ -63,13 +65,11 @@ public class PojoMessageHandlerWholeBinary
if (decoderClazzes != null) {
for (Class<? extends Decoder> decoderClazz : decoderClazzes) {
if (Binary.class.isAssignableFrom(decoderClazz)) {
- Binary<?> decoder = (Binary<?>)
decoderClazz.getConstructor().newInstance();
+ Binary<?> decoder = (Binary<?>)
createDecoderInstance(decoderClazz);
decoder.init(config);
decoders.add(decoder);
- } else if (BinaryStream.class.isAssignableFrom(
- decoderClazz)) {
- BinaryStream<?> decoder = (BinaryStream<?>)
- decoderClazz.getConstructor().newInstance();
+ } else if
(BinaryStream.class.isAssignableFrom(decoderClazz)) {
+ BinaryStream<?> decoder = (BinaryStream<?>)
createDecoderInstance(decoderClazz);
decoder.init(config);
decoders.add(decoder);
} else {
@@ -77,7 +77,7 @@ public class PojoMessageHandlerWholeBinary
}
}
}
- } catch (ReflectiveOperationException e) {
+ } catch (ReflectiveOperationException | NamingException e) {
throw new IllegalArgumentException(e);
}
this.isForInputStream = isForInputStream;
diff --git
a/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeText.java
b/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeText.java
index ce12f16..9d2c646 100644
--- a/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeText.java
+++ b/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeText.java
@@ -21,6 +21,8 @@ import java.io.StringReader;
import java.lang.reflect.Method;
import java.util.List;
+import javax.naming.NamingException;
+
import jakarta.websocket.DecodeException;
import jakarta.websocket.Decoder;
import jakarta.websocket.Decoder.Text;
@@ -73,13 +75,11 @@ public class PojoMessageHandlerWholeText
if (decoderClazzes != null) {
for (Class<? extends Decoder> decoderClazz : decoderClazzes) {
if (Text.class.isAssignableFrom(decoderClazz)) {
- Text<?> decoder = (Text<?>)
decoderClazz.getConstructor().newInstance();
+ Text<?> decoder = (Text<?>)
createDecoderInstance(decoderClazz);
decoder.init(config);
decoders.add(decoder);
- } else if (TextStream.class.isAssignableFrom(
- decoderClazz)) {
- TextStream<?> decoder =
- (TextStream<?>)
decoderClazz.getConstructor().newInstance();
+ } else if
(TextStream.class.isAssignableFrom(decoderClazz)) {
+ TextStream<?> decoder = (TextStream<?>)
createDecoderInstance(decoderClazz);
decoder.init(config);
decoders.add(decoder);
} else {
@@ -87,7 +87,7 @@ public class PojoMessageHandlerWholeText
}
}
}
- } catch (ReflectiveOperationException e) {
+ } catch (ReflectiveOperationException | NamingException e) {
throw new IllegalArgumentException(e);
}
}
diff --git a/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
b/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
index 4ace70f..ce07c34 100644
--- a/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
+++ b/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
@@ -44,6 +44,7 @@ import jakarta.websocket.PongMessage;
import jakarta.websocket.Session;
import jakarta.websocket.server.PathParam;
+import org.apache.tomcat.InstanceManager;
import org.apache.tomcat.util.res.StringManager;
import org.apache.tomcat.websocket.DecoderEntry;
import org.apache.tomcat.websocket.Util;
@@ -70,13 +71,41 @@ public class PojoMethodMapping {
private final String wsPath;
- public PojoMethodMapping(Class<?> clazzPojo,
- List<Class<? extends Decoder>> decoderClazzes, String wsPath)
- throws DeploymentException {
+ /**
+ * Create a method mapping for the given POJO
+ *
+ * @param clazzPojo POJO implementation class
+ * @param decoderClazzes Set of potential decoder classes
+ * @param wsPath Path at which the endpoint will be deployed
+ *
+ * @throws DeploymentException If the mapping cannot be completed
+ *
+ * @deprecated Will be removed in Tomcat 10.1.x
+ * Use (@link {@link #PojoMethodMapping(Class, List, String,
InstanceManager)}
+ */
+ @Deprecated
+ public PojoMethodMapping(Class<?> clazzPojo, List<Class<? extends
Decoder>> decoderClazzes, String wsPath)
+ throws DeploymentException {
+ this(clazzPojo, decoderClazzes, wsPath, null);
+ }
+
+
+ /**
+ * Create a method mapping for the given POJO
+ *
+ * @param clazzPojo POJO implementation class
+ * @param decoderClazzes Set of potential decoder classes
+ * @param wsPath Path at which the endpoint will be deployed
+ * @param instanceManager Instance manager to use to create Decoder
instances
+ *
+ * @throws DeploymentException If the mapping cannot be completed
+ */
+ public PojoMethodMapping(Class<?> clazzPojo, List<Class<? extends
Decoder>> decoderClazzes, String wsPath,
+ InstanceManager instanceManager) throws DeploymentException {
this.wsPath = wsPath;
- List<DecoderEntry> decoders = Util.getDecoders(decoderClazzes);
+ List<DecoderEntry> decoders = Util.getDecoders(decoderClazzes,
instanceManager);
Method open = null;
Method close = null;
Method error = null;
diff --git a/java/org/apache/tomcat/websocket/server/WsServerContainer.java
b/java/org/apache/tomcat/websocket/server/WsServerContainer.java
index 40d52cc..96c1422 100644
--- a/java/org/apache/tomcat/websocket/server/WsServerContainer.java
+++ b/java/org/apache/tomcat/websocket/server/WsServerContainer.java
@@ -151,7 +151,7 @@ public class WsServerContainer extends WsWebSocketContainer
// Add method mapping to user properties
PojoMethodMapping methodMapping = new
PojoMethodMapping(sec.getEndpointClass(),
- sec.getDecoders(), path);
+ sec.getDecoders(), path,
getInstanceManager(Thread.currentThread().getContextClassLoader()));
if (methodMapping.getOnClose() != null ||
methodMapping.getOnOpen() != null
|| methodMapping.getOnError() != null ||
methodMapping.hasMessageHandlers()) {
sec.getUserProperties().put(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY,
diff --git a/test/org/apache/tomcat/websocket/TesterWsClientAutobahn.java
b/test/org/apache/tomcat/websocket/TesterWsClientAutobahn.java
index 4917004..79c5e2a 100644
--- a/test/org/apache/tomcat/websocket/TesterWsClientAutobahn.java
+++ b/test/org/apache/tomcat/websocket/TesterWsClientAutobahn.java
@@ -99,7 +99,7 @@ public class TesterWsClientAutobahn {
List<Extension> extensions = new ArrayList<>(1);
extensions.add(permessageDeflate);
- Endpoint ep = new PojoEndpointClient(testCaseClient, null);
+ Endpoint ep = new PojoEndpointClient(testCaseClient, null, null);
ClientEndpointConfig.Builder builder =
ClientEndpointConfig.Builder.create();
ClientEndpointConfig config = builder.extensions(extensions).build();
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
