diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyProtocolClientConnectionFactory.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyProtocolClientConnectionFactory.java index 4eef6cfdb796..e4ba9bfcc233 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyProtocolClientConnectionFactory.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyProtocolClientConnectionFactory.java @@ -508,12 +508,6 @@ public void failed(Throwable x) promise.failed(x); } - @Override - public InvocationType getInvocationType() - { - return InvocationType.NON_BLOCKING; - } - @Override public void onFillable() { diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java index aa550df9c574..e757bbc131d2 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java @@ -34,6 +34,7 @@ import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.util.Attributes; +import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.thread.ThreadPool; import org.slf4j.Logger; @@ -43,6 +44,7 @@ public class ServerFCGIConnection extends AbstractMetaDataConnection implements { private static final Logger LOG = LoggerFactory.getLogger(ServerFCGIConnection.class); + private final Callback fillableCallback = new FillableCallback(); private final HttpChannel.Factory httpChannelFactory = new HttpChannel.DefaultFactory(); private final Attributes attributes = new Lazy(); private final Connector connector; @@ -161,7 +163,7 @@ public void clearAttributes() public void onOpen() { super.onOpen(); - fillInterested(); + fillInterested(fillableCallback); } @Override @@ -189,7 +191,7 @@ public void onFillable() else if (read == 0) { releaseInputBuffer(); - fillInterested(); + fillInterested(fillableCallback); return; } else @@ -305,7 +307,7 @@ void onCompleted(Throwable failure) { releaseInputBuffer(); if (failure == null) - fillInterested(); + fillInterested(fillableCallback); else getFlusher().shutdown(); } @@ -407,25 +409,46 @@ public void onFailure(int request, Throwable failure) @Override public void close() { - if (stream != null) + try { - Runnable task = stream.getHttpChannel().onClose(); - if (task != null) + if (stream != null) { - ThreadPool.executeImmediately(getExecutor(), () -> - { - try - { - task.run(); - } - finally - { - super.close(); - } - }); - return; + Runnable task = stream.getHttpChannel().onClose(); + if (task != null) + task.run(); } } - super.close(); + finally + { + super.close(); + } + } + + private class FillableCallback implements Callback + { + private final InvocationType invocationType; + + public FillableCallback() + { + invocationType = getConnector().getServer().getInvocationType(); + } + + @Override + public void succeeded() + { + onFillable(); + } + + @Override + public void failed(Throwable x) + { + onFillInterestedFailed(x); + } + + @Override + public InvocationType getInvocationType() + { + return invocationType; + } } } diff --git a/jetty-core/jetty-http2/jetty-http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java b/jetty-core/jetty-http2/jetty-http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java index 289d8bce0245..c9487fde74cd 100644 --- a/jetty-core/jetty-http2/jetty-http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java +++ b/jetty-core/jetty-http2/jetty-http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java @@ -174,12 +174,6 @@ public void failed(Throwable x) close(); promise.failed(x); } - - @Override - public InvocationType getInvocationType() - { - return InvocationType.NON_BLOCKING; - } } private static class ConnectionListener implements Connection.Listener diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java index 3d6f03dca956..c421acac6deb 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java @@ -432,7 +432,7 @@ else if (filled == 0) } networkBuffer = null; if (interested) - getEndPoint().fillInterested(fillableCallback); + fillInterested(fillableCallback); } } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java index df299d5e349b..594914174625 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java @@ -51,15 +51,6 @@ protected AbstractConnection(EndPoint endPoint, Executor executor) _readCallback = new ReadCallback(); } - @Deprecated - @Override - public InvocationType getInvocationType() - { - // TODO consider removing the #fillInterested method from the connection and only use #fillInterestedCallback - // so a connection need not be Invocable - return Invocable.super.getInvocationType(); - } - @Override public void addEventListener(EventListener listener) { @@ -90,25 +81,27 @@ protected Executor getExecutor() } /** - *

Utility method to be called to register read interest.

- *

After a call to this method, {@link #onFillable()} or {@link #onFillInterestedFailed(Throwable)} - * will be called back as appropriate.

+ *

Registers read interest using the default {@link Callback} with {@link Invocable.InvocationType#BLOCKING}.

+ *

When read readiness is signaled, {@link #onFillable()} or {@link #onFillInterestedFailed(Throwable)} + * will be invoked.

+ *

This method should be used sparingly, mainly from {@link #onOpen()}, and {@link #fillInterested(Callback)} + * should be preferred instead, passing a {@link Callback} that specifies the {@link Invocable.InvocationType} + * for each specific case where read interest needs to be registered.

* + * @see #fillInterested(Callback) * @see #onFillable() + * @see #onFillInterestedFailed(Throwable) */ public void fillInterested() { - if (LOG.isDebugEnabled()) - LOG.debug("fillInterested {}", this); - getEndPoint().fillInterested(_readCallback); + fillInterested(_readCallback); } /** - *

Utility method to be called to register read interest.

- *

After a call to this method, {@link #onFillable()} or {@link #onFillInterestedFailed(Throwable)} - * will be called back as appropriate.

+ *

Registers read interest with the given callback.

+ *

When read readiness is signaled, the callback will be completed.

* - * @see #onFillable() + * @param callback the callback to complete when read readiness is signaled */ public void fillInterested(Callback callback) { @@ -130,7 +123,7 @@ public boolean isFillInterested() /** *

Callback method invoked when the endpoint is ready to be read.

* - * @see #fillInterested() + * @see #fillInterested(Callback) */ public abstract void onFillable(); @@ -139,7 +132,7 @@ public boolean isFillInterested() * * @param cause the exception that caused the failure */ - protected void onFillInterestedFailed(Throwable cause) + public void onFillInterestedFailed(Throwable cause) { if (LOG.isDebugEnabled()) LOG.debug("onFillInterestedFailed {}", this, cause); @@ -286,7 +279,12 @@ public String toConnectionString() return String.format("%s@%x", getClass().getSimpleName(), hashCode()); } - private class ReadCallback implements Callback, Invocable + /** + *

The default {@link Callback} for read interest, typically used from {@link #onOpen()}.

+ *

In other cases, use {@link #fillInterested(Callback)} with a {@link Callback} that + * reports a more specific {@link Invocable.InvocationType}.

+ */ + private class ReadCallback implements Callback { @Override public void succeeded() @@ -305,11 +303,5 @@ public String toString() { return String.format("%s@%x{%s}", getClass().getSimpleName(), hashCode(), AbstractConnection.this); } - - @Override - public InvocationType getInvocationType() - { - return AbstractConnection.this.getInvocationType(); - } } } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java index 94e60222ccac..c85751bd9b8f 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java @@ -88,6 +88,7 @@ public class HttpConnection extends AbstractMetaDataConnection implements Runnab private static final ThreadLocal __currentConnection = new ThreadLocal<>(); private static final AtomicLong __connectionIdGenerator = new AtomicLong(); + private final Callback _fillableCallback = new FillableCallback(); private final TunnelSupport _tunnelSupport = new TunnelSupportOverHTTP1(); private final AtomicLong _streamIdGenerator = new AtomicLong(); private final long _id; @@ -146,12 +147,6 @@ public HttpConnection(HttpConfiguration configuration, Connector connector, EndP LOG.debug("New HTTP Connection {}", this); } - @Override - public InvocationType getInvocationType() - { - return getServer().getInvocationType(); - } - protected HttpGenerator newHttpGenerator() { HttpGenerator generator = new HttpGenerator(); @@ -451,7 +446,7 @@ else if (filled == 0) { assert isRequestBufferEmpty(); releaseRequestBuffer(); - fillInterested(); + fillInterested(_fillableCallback); break; } else if (filled < 0) @@ -623,7 +618,7 @@ private boolean upgrade(HttpStreamOverHTTP1 stream) } @Override - protected void onFillInterestedFailed(Throwable cause) + public void onFillInterestedFailed(Throwable cause) { _parser.close(); super.onFillInterestedFailed(cause); @@ -641,10 +636,16 @@ public boolean onIdleExpired(TimeoutException timeout) @Override public void close() { - Runnable task = _httpChannel.onClose(); - if (task != null) - task.run(); - super.close(); + try + { + Runnable task = _httpChannel.onClose(); + if (task != null) + task.run(); + } + finally + { + super.close(); + } } @Override @@ -652,7 +653,7 @@ public void onOpen() { super.onOpen(); if (isRequestBufferEmpty()) - fillInterested(); + fillInterested(_fillableCallback); else getExecutor().execute(this); } @@ -1678,4 +1679,32 @@ public String getReason() return getMessage(); } } + + private class FillableCallback implements Callback + { + private final InvocationType _invocationType; + + public FillableCallback() + { + _invocationType = getServer().getInvocationType(); + } + + @Override + public void succeeded() + { + onFillable(); + } + + @Override + public void failed(Throwable x) + { + onFillInterestedFailed(x); + } + + @Override + public InvocationType getInvocationType() + { + return _invocationType; + } + } } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/Invocable.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/Invocable.java index 504115b5ed0e..26ebfd861d7e 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/Invocable.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/Invocable.java @@ -63,8 +63,8 @@ public void runWithoutBlocking(Runnable task, Executor executor) /** *

Invoking the task does not block the invoker thread, * and the invocation must be performed immediately in the invoker thread.

- *

This invocation type is suitable for tasks that can not be deferred and is - * guaranteed to never block the invoker thread.

+ *

This invocation type is suitable for tasks that cannot be deferred and + * guarantee that their code never blocks the invoker thread.

*/ NON_BLOCKING { @@ -74,20 +74,20 @@ public void runWithoutBlocking(Runnable task, Executor ignored) } }, /** - *

Invoking the task may act either as a {@code BLOCKING} task if invoked directly; or as a {@code NON_BLOCKING} - * task if invoked via {@link Invocable#invokeNonBlocking(Runnable)}. The implementation of the task must check - * {@link Invocable#isNonBlockingInvocation()} to determine how it was called. - *

- *

This invocation type is suitable for tasks that have multiple subtasks, some of which that cannot be deferred - * mixed with other subtasks that can be. - * An invoker which has an {@code EITHER} task must call it immediately, either directly, so that it may block; or - * via {@link Invocable#invokeNonBlocking(Runnable)} so that it may not. + *

Invoking the task may act either as a {@code BLOCKING} task if invoked directly; + * or as a {@code NON_BLOCKING} task if invoked via {@link Invocable#invokeNonBlocking(Runnable)}.

+ *

The implementation of the task must check {@link Invocable#isNonBlockingInvocation()} + * to determine how it was called.

+ *

This invocation type is suitable for tasks that have multiple subtasks, + * some of which that cannot be deferred mixed with other subtasks that can be. + * An invoker which has an {@code EITHER} task must call it immediately, either + * directly, so that it may block; or via {@link Invocable#invokeNonBlocking(Runnable)} + * so that it may not. * The invoker cannot defer the task execution, and specifically it must not - * queue the {@code EITHER} task in a thread pool. - *

- *

See the {@link org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy} for an example of - * both an invoker of {@code EITHER} tasks, and as an implementation of an {@code EITHER} task, when used in a - * chain of {@link ExecutionStrategy}s.

+ * queue the {@code EITHER} task in a thread pool.

+ *

See the {@link org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy} + * for an example of both an invoker of {@code EITHER} tasks, and as an implementation + * of an {@code EITHER} task, when used in a chain of {@link ExecutionStrategy}s.

*/ EITHER { @@ -98,23 +98,24 @@ public void runWithoutBlocking(Runnable task, Executor ignored) }; /** - * Run or Execute the task according to the InvocationType without blocking the caller: + *

Runs or executes the task according to the {@code InvocationType}, + * without blocking the caller:

*
*
{@link InvocationType#NON_BLOCKING}
*
The task is run directly
*
{@link InvocationType#BLOCKING}
- *
The task is executed by the passed executor
+ *
The task is submitted to the given {@link Executor}
*
{@link InvocationType#EITHER}
*
The task is invoked via {@link Invocable#invokeNonBlocking(Runnable)}
*
* @param task The task to run - * @param executor The executor to use if necessary + * @param executor The {@link Executor} to use if necessary */ public abstract void runWithoutBlocking(Runnable task, Executor executor); } /** - *

A task with an {@link InvocationType}.

+ *

A {@link Runnable} task with an {@link InvocationType}.

*/ interface Task extends Invocable, Runnable { @@ -178,7 +179,7 @@ public void run() @Override public String toString() { - return String.format("%s@%x[%s|%s]", getClass().getSimpleName(), hashCode(), getInvocationType(), task); + return String.format("%s[%s]", super.toString(), task); } } @@ -197,7 +198,7 @@ static Task from(InvocationType type, Runnable task) } /** - * Test if the current thread has been tagged as non blocking + * Tests if the current thread has been tagged as non-blocking. * * @return True if the task the current thread is running has * indicated that it will not block. @@ -231,7 +232,7 @@ static void invokeNonBlocking(Runnable task) * Combine two invocation type. * @param it1 A type * @param it2 Another type - * @return The combination of both type, where any tendency to block overrules any non blocking. + * @return The combination of both type, where any tendency to block overrules any non-blocking. */ static InvocationType combine(InvocationType it1, InvocationType it2) { @@ -258,7 +259,7 @@ static InvocationType combineTypes(InvocationType... it) } /** - * Get the invocation type of an Object. + * Get the {@code InvocationType} of the given object. * * @param o The object to check the invocation type of. * @return If the object is an Invocable, it is coerced and the {@link #getInvocationType()}