On Tue, May 24, 2022 at 6:46 PM <ma...@apache.org> wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch 9.0.x
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/9.0.x by this push:
> new 680db4448d Additional fixes for 66076
> 680db4448d is described below
>
> commit 680db4448d392752fb57b91639e7ab34a3f58105
> Author: Mark Thomas <ma...@apache.org>
> AuthorDate: Tue May 24 17:46:20 2022 +0100
>
> Additional fixes for 66076
>
> The vectored IO version of the fix.
> ---
> java/org/apache/tomcat/util/net/AprEndpoint.java | 7 +++++++
> java/org/apache/tomcat/util/net/Nio2Endpoint.java | 7 +++++++
> java/org/apache/tomcat/util/net/NioEndpoint.java | 19
> ++++++++++++-------
> .../org/apache/tomcat/util/net/SocketWrapperBase.java | 4 +++-
> 4 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java
> b/java/org/apache/tomcat/util/net/AprEndpoint.java
> index c02e90fc09..20d10efa11 100644
> --- a/java/org/apache/tomcat/util/net/AprEndpoint.java
> +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
> @@ -2813,6 +2813,13 @@ public class AprEndpoint extends
> AbstractEndpoint<Long,Long> implements SNICallB
> return inline;
> }
>
> + @Override
> + protected boolean hasOutboundRemaining() {
> + // NIO2 never has remaining outbound data when the completion
> + // handler is called
> + return false;
> + }
> +
> @Override
> public void run() {
> // Perform the IO operation
> diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
> b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
> index 49ee411016..c5b9a395f5 100644
> --- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
> +++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
> @@ -1021,6 +1021,13 @@ public class Nio2Endpoint extends
> AbstractJsseEndpoint<Nio2Channel,AsynchronousS
> return Nio2Endpoint.isInline();
> }
>
> + @Override
> + protected boolean hasOutboundRemaining() {
> + // NIO2 never has remaining outbound data when the completion
> + // handler is called
> + return false;
> + }
> +
> @Override
> protected void start() {
> if (read) {
> diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java
> b/java/org/apache/tomcat/util/net/NioEndpoint.java
> index b8b6d4339d..ea65dd6600 100644
> --- a/java/org/apache/tomcat/util/net/NioEndpoint.java
> +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
> @@ -1622,6 +1622,11 @@ public class NioEndpoint extends
> AbstractJsseEndpoint<NioChannel,SocketChannel>
> return inline;
> }
>
> + @Override
> + protected boolean hasOutboundRemaining() {
> + return getSocket().getOutboundRemaining() > 0;
> + }
> +
> @Override
> public void run() {
> // Perform the IO operation
> @@ -1654,13 +1659,13 @@ public class NioEndpoint extends
> AbstractJsseEndpoint<NioChannel,SocketChannel>
> } else {
> boolean doWrite = true;
> // Write from main buffer first
> - if
> (!socketBufferHandler.isWriteBufferEmpty()) {
> + if (socketOrNetworkBufferHasDataLeft()) {
> // There is still data inside the main
> write buffer, it needs to be written first
>
> socketBufferHandler.configureWriteBufferForRead();
> do {
> nBytes =
> getSocket().write(socketBufferHandler.getWriteBuffer());
> - } while
> (!socketBufferHandler.isWriteBufferEmpty() && nBytes > 0);
> - if
> (!socketBufferHandler.isWriteBufferEmpty()) {
> + } while
> (socketOrNetworkBufferHasDataLeft() && nBytes > 0);
> + if (socketOrNetworkBufferHasDataLeft()) {
> doWrite = false;
> }
> // Preserve a negative value since it is
> an error
> @@ -1681,7 +1686,8 @@ public class NioEndpoint extends
> AbstractJsseEndpoint<NioChannel,SocketChannel>
> updateLastWrite();
> }
> }
> - if (nBytes != 0 ||
> !buffersArrayHasRemaining(buffers, offset, length)) {
> + if (nBytes != 0 ||
> (!buffersArrayHasRemaining(buffers, offset, length) &&
> + !socketOrNetworkBufferHasDataLeft())) {
> completionDone = false;
> }
> }
> @@ -1689,7 +1695,8 @@ public class NioEndpoint extends
> AbstractJsseEndpoint<NioChannel,SocketChannel>
> setError(e);
> }
> }
> - if (nBytes > 0 || (nBytes == 0 &&
> !buffersArrayHasRemaining(buffers, offset, length))) {
> + if (nBytes > 0 || (nBytes == 0 &&
> !buffersArrayHasRemaining(buffers, offset, length) &&
> + !socketOrNetworkBufferHasDataLeft())) {
> // The bytes processed are only updated in the
> completion handler
> completion.completed(Long.valueOf(nBytes), this);
> } else if (nBytes < 0 || getError() != null) {
> @@ -1708,9 +1715,7 @@ public class NioEndpoint extends
> AbstractJsseEndpoint<NioChannel,SocketChannel>
> }
> }
> }
> -
> }
> -
> }
>
>
> diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
> b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
> index bcb0119fc6..d95a31a15d 100644
> --- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
> +++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
> @@ -1036,6 +1036,8 @@ public abstract class SocketWrapperBase<E> {
> */
> protected abstract boolean isInline();
>
> + protected abstract boolean hasOutboundRemaining();
> +
> /**
> * Process the operation using the connector executor.
> * @return true if the operation was accepted, false if the executor
> @@ -1087,7 +1089,7 @@ public abstract class SocketWrapperBase<E> {
> boolean completion = true;
> if (state.check != null) {
> CompletionHandlerCall call =
> state.check.callHandler(currentState, state.buffers, state.offset,
> state.length);
> - if (call == CompletionHandlerCall.CONTINUE) {
> + if (call == CompletionHandlerCall.CONTINUE ||
> state.hasOutboundRemaining()) {
I understand why it's there (the operation will indeed seem complete
in that case, but it's not fully complete), but it needs to check
state.read then (looping a read operation will most likely not do
anything good; maybe it shouldn't happen, but it's a lot better to be
safe), so:
if (call == CompletionHandlerCall.CONTINUE || (!state.read &&
state.hasOutboundRemaining())) {
Overall the change is more convoluted than I expected, and I would
have messed it up without a test case.
Rémy
> complete = false;
> } else if (call == CompletionHandlerCall.NONE) {
> completion = false;
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
