From 72ba95d34f2794fb1ba8fa154066abd0466285b5 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 8 Jun 2022 11:43:59 +1000 Subject: [PATCH] Issue #6328 - changes from review Signed-off-by: Lachlan Roberts --- .../messages/ByteBufferMessageSink.java | 7 --- .../core/internal/util/MethodHolder.java | 2 +- .../internal/util/NonBindingMethodHolder.java | 55 ++++++------------- .../JavaxWebSocketFrameHandlerFactory.java | 2 +- .../common/OutgoingMessageCapture.java | 2 +- 5 files changed, 21 insertions(+), 47 deletions(-) diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/messages/ByteBufferMessageSink.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/messages/ByteBufferMessageSink.java index a297509a7111..f7b5fb503e0d 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/messages/ByteBufferMessageSink.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/messages/ByteBufferMessageSink.java @@ -13,7 +13,6 @@ package org.eclipse.jetty.websocket.core.internal.messages; -import java.lang.invoke.MethodHandle; import java.nio.ByteBuffer; import org.eclipse.jetty.io.ByteBufferCallbackAccumulator; @@ -34,12 +33,6 @@ public ByteBufferMessageSink(CoreSession session, MethodHolder methodHolder) super(session, methodHolder); } - @Deprecated - public ByteBufferMessageSink(CoreSession session, MethodHandle methodHandle) - { - this(session, MethodHolder.from(methodHandle)); - } - @Override public void accept(Frame frame, Callback callback) { diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/util/MethodHolder.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/util/MethodHolder.java index c18ca0da30d0..b72733fbc9f4 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/util/MethodHolder.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/util/MethodHolder.java @@ -32,7 +32,7 @@ */ public interface MethodHolder { - String METHOD_HOLDER_BINDING_PROPERTY = "jetty.methodholder.binding"; + String METHOD_HOLDER_BINDING_PROPERTY = "jetty.websocket.methodholder.binding"; static MethodHolder from(MethodHandle methodHandle) { diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/util/NonBindingMethodHolder.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/util/NonBindingMethodHolder.java index 209b8ea198a4..fbdfc307f950 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/util/NonBindingMethodHolder.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/util/NonBindingMethodHolder.java @@ -22,14 +22,15 @@ /** * This implementation of {@link MethodHolder} is not thread safe. - * Mutual exclusion should be used when calling {@link #invoke(Object...)}. + * Mutual exclusion should be used when calling {@link #invoke(Object...)}, or this should only + * be invoked from a single thread. */ class NonBindingMethodHolder implements MethodHolder { private final MethodHandle _methodHandle; private final Object[] _parameters; - private final List _lookupTable = new LinkedList<>(); - private List _returnFilters; + private final List _unboundParamIndexes = new LinkedList<>(); + private final List _returnFilters = new ArrayList<>(); public NonBindingMethodHolder(MethodHandle methodHandle) { @@ -38,24 +39,10 @@ public NonBindingMethodHolder(MethodHandle methodHandle) _parameters = new Object[numParams]; for (int i = 0; i < numParams; i++) { - _lookupTable.add(i); + _unboundParamIndexes.add(i); } } - private void dropFromLookupTable(int index) - { - if (index < 0 || index >= _lookupTable.size()) - throw new IndexOutOfBoundsException(); - _lookupTable.remove(index); - } - - private int getInternalIndex(int index) - { - if (index < 0 || index >= _lookupTable.size()) - throw new IndexOutOfBoundsException(); - return _lookupTable.get(index); - } - @Override public Object invoke(Object... args) throws Throwable { @@ -63,12 +50,9 @@ public Object invoke(Object... args) throws Throwable { insertArguments(args); Object o = _methodHandle.invokeWithArguments(_parameters); - if (_returnFilters != null) + for (MethodHandle filter : _returnFilters) { - for (MethodHandle filter : _returnFilters) - { - o = filter.invoke(o); - } + o = filter.invoke(o); } return o; } @@ -81,8 +65,8 @@ public Object invoke(Object... args) throws Throwable @Override public MethodHolder bindTo(Object arg, int idx) { - _parameters[getInternalIndex(idx)] = arg; - dropFromLookupTable(idx); + _parameters[_unboundParamIndexes.get(idx)] = arg; + _unboundParamIndexes.remove(idx); return this; } @@ -92,22 +76,21 @@ public MethodHolder bindTo(Object arg) return bindTo(arg, 0); } - @SuppressWarnings({"UnusedReturnValue", "SameParameterValue"}) - private MethodHolder insertArguments(Object... args) + private void insertArguments(Object... args) { - if (_lookupTable.size() != args.length) - throw new WrongMethodTypeException(String.format("Expected %s params but had %s", _lookupTable.size(), args.length)); + if (_unboundParamIndexes.size() != args.length) + throw new WrongMethodTypeException(String.format("Expected %s params but had %s", _unboundParamIndexes.size(), args.length)); - for (int i = 0; i < args.length; i++) + int argsIndex = 0; + for (int index : _unboundParamIndexes) { - _parameters[_lookupTable.get(i)] = args[i]; + _parameters[index] = args[argsIndex++]; } - return this; } private void clearArguments() { - for (int i : _lookupTable) + for (int i : _unboundParamIndexes) { _parameters[i] = null; } @@ -116,8 +99,6 @@ private void clearArguments() @Override public MethodHolder filterReturnValue(MethodHandle filter) { - if (_returnFilters == null) - _returnFilters = new ArrayList<>(); _returnFilters.add(filter); return this; } @@ -125,12 +106,12 @@ public MethodHolder filterReturnValue(MethodHandle filter) @Override public Class parameterType(int idx) { - return _methodHandle.type().parameterType(getInternalIndex(idx)); + return _methodHandle.type().parameterType(_unboundParamIndexes.get(idx)); } @Override public Class returnType() { - return ((_returnFilters == null) ? _methodHandle : _returnFilters.get(_returnFilters.size() - 1)).type().returnType(); + return (_returnFilters.isEmpty() ? _methodHandle : _returnFilters.get(_returnFilters.size() - 1)).type().returnType(); } } diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandlerFactory.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandlerFactory.java index ac262aa9e0cb..651d9ce00117 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandlerFactory.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandlerFactory.java @@ -205,7 +205,7 @@ public static MessageSink createMessageSink(JavaxWebSocketSession session, Javax } } - public static MethodHolder wrapNonVoidReturnType(MethodHolder handle, JavaxWebSocketSession session) + static MethodHolder wrapNonVoidReturnType(MethodHolder handle, JavaxWebSocketSession session) { if (handle == null) return null; diff --git a/jetty-websocket/websocket-jetty-common/src/test/java/org/eclipse/jetty/websocket/common/OutgoingMessageCapture.java b/jetty-websocket/websocket-jetty-common/src/test/java/org/eclipse/jetty/websocket/common/OutgoingMessageCapture.java index 80020f9a386f..b22d6596fd6e 100644 --- a/jetty-websocket/websocket-jetty-common/src/test/java/org/eclipse/jetty/websocket/common/OutgoingMessageCapture.java +++ b/jetty-websocket/websocket-jetty-common/src/test/java/org/eclipse/jetty/websocket/common/OutgoingMessageCapture.java @@ -106,7 +106,7 @@ public void sendFrame(Frame frame, Callback callback, boolean batch) String event = String.format("BINARY:fin=%b:len=%d", frame.isFin(), frame.getPayloadLength()); LOG.debug(event); events.offer(event); - messageSink = new ByteBufferMessageSink(this, wholeBinaryHandle); + messageSink = new ByteBufferMessageSink(this, MethodHolder.from(wholeBinaryHandle)); break; } case OpCode.CONTINUATION: