You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2022/12/12 15:43:43 UTC
[tomcat] branch main updated: Add support for Servlet read/write via ByteBuffer
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 2d931eed6c Add support for Servlet read/write via ByteBuffer
2d931eed6c is described below
commit 2d931eed6c35702ef6541c0fd6c067b79e5eb371
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Dec 12 15:43:28 2022 +0000
Add support for Servlet read/write via ByteBuffer
---
java/jakarta/servlet/ServletInputStream.java | 141 ++++++++++++++++++++-
java/jakarta/servlet/ServletOutputStream.java | 101 ++++++++++++++-
.../catalina/connector/CoyoteInputStream.java | 14 +-
.../catalina/connector/CoyoteOutputStream.java | 6 +
.../org/apache/catalina/connector/InputBuffer.java | 4 +
.../catalina/connector/LocalStrings.properties | 1 +
java/org/apache/coyote/LocalStrings.properties | 1 -
java/org/apache/coyote/Response.java | 5 +-
.../apache/coyote/http11/Http11OutputBuffer.java | 7 +
webapps/docs/changelog.xml | 11 ++
10 files changed, 264 insertions(+), 27 deletions(-)
diff --git a/java/jakarta/servlet/ServletInputStream.java b/java/jakarta/servlet/ServletInputStream.java
index 0c8280c2e9..10048fbb88 100644
--- a/java/jakarta/servlet/ServletInputStream.java
+++ b/java/jakarta/servlet/ServletInputStream.java
@@ -18,6 +18,8 @@ package jakarta.servlet;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.ByteBuffer;
+import java.util.Objects;
/**
* Provides an input stream for reading binary data from a client request,
@@ -43,6 +45,80 @@ public abstract class ServletInputStream extends InputStream {
// NOOP
}
+ /**
+ * Reads from the input stream into the given buffer.
+ * <p>
+ * If the input stream is in non-blocking mode, before each invocation of
+ * this method {@link #isReady()} must be called and must return
+ * {@code true} or the {@link ReadListener#onDataAvailable()} call back must
+ * indicate that data is available to read else an
+ * {@link IllegalStateException} must be thrown.
+ * <p>
+ * Otherwise, if this method is called when {@code buffer} has no space
+ * remaining, the method returns {@code 0} immediately and {@code buffer} is
+ * unchanged.
+ * <p>
+ * If the input stream is in blocking mode and {@code buffer} has space
+ * remaining, this method blocks until at least one byte has been read, end
+ * of stream is reached or an exception is thrown.
+ * <p>
+ * Returns the number of bytes read or {@code -1} if the end of stream is
+ * reached without reading any data.
+ * <p>
+ * When the method returns, and if data has been read, the buffer's position
+ * will be unchanged from the value when passed to this method and the limit
+ * will be the position incremented by the number of bytes read.
+ * <p>
+ * Subclasses are strongly encouraged to override this method and provide a
+ * more efficient implementation.
+ *
+ * @param buffer The buffer into which the data is read.
+ *
+ * @return The number of bytes read or {@code -1} if the end of the stream
+ * has been reached.
+ *
+ * @exception IllegalStateException If the input stream is in non-blocking
+ * mode and this method is called without first calling {@link #isReady()}
+ * and that method has returned {@code true} or
+ * {@link ReadListener#onDataAvailable()} has not signalled that data is
+ * available to read.
+ *
+ * @exception IOException If data cannot be read for any reason other than
+ * the end of stream being reached, the input stream has been closed or if
+ * some other I/O error occurs.
+ *
+ * @exception NullPointerException If buffer is null.
+ *
+ * @since Servlet 6.1
+ */
+ public int read(ByteBuffer buffer) throws IOException {
+ Objects.requireNonNull(buffer);
+
+ if (!isReady()) {
+ throw new IllegalStateException();
+ }
+
+ if (buffer.remaining() == 0) {
+ return 0;
+ }
+
+ byte[] b = new byte[buffer.remaining()];
+
+ int result = read(b);
+ if (result == -1) {
+ return -1;
+ }
+
+ int position = buffer.position();
+
+ buffer.put(b, 0, result);
+
+ buffer.position(position);
+ buffer.limit(position + result);
+
+ return result;
+ }
+
/**
* Reads the input stream, one line at a time. Starting at an offset, reads
* bytes into an array, until it reads a certain number of bytes or reaches
@@ -50,6 +126,8 @@ public abstract class ServletInputStream extends InputStream {
* <p>
* This method returns -1 if it reaches the end of the input stream before
* reading the maximum number of bytes.
+ * <p>
+ * This method may only be used when the input stream is in blocking mode.
*
* @param b
* an array of bytes into which data is read
@@ -60,6 +138,10 @@ public abstract class ServletInputStream extends InputStream {
* an integer specifying the maximum number of bytes to read
* @return an integer specifying the actual number of bytes read, or -1 if
* the end of the stream is reached
+ *
+ * @exception IllegalStateException
+ * If this method is called when the input stream is in
+ * non-blocking mode.
* @exception IOException
* if an input or output exception has occurred
*/
@@ -91,12 +173,25 @@ public abstract class ServletInputStream extends InputStream {
public abstract boolean isFinished();
/**
- * Can data be read from this InputStream without blocking?
- * Returns If this method is called and returns false, the container will
- * invoke {@link ReadListener#onDataAvailable()} when data is available.
+ * Returns {@code true} if it is allowable to call a {@code read()} method.
+ * In blocking mode, this method will always return {@code true}, but a
+ * subsequent call to a {@code read()} method may block awaiting data. In
+ * non-blocking mode this method may return {@code false}, in which case it
+ * is illegal to call a {@code read()} method and an
+ * {@link IllegalStateException} MUST be thrown. When
+ * {@link ReadListener#onDataAvailable()} is called, a call to this method
+ * that returned {@code true} is implicit.
+ * <p>
+ * If this method returns {@code false} and a {@link ReadListener} has been
+ * set via {@link #setReadListener(ReadListener)}, then the container will
+ * subsequently invoke {@link ReadListener#onDataAvailable()} (or
+ * {@link ReadListener#onAllDataRead()}) once data (or EOF) has become
+ * available. Other than the initial call
+ * {@link ReadListener#onDataAvailable()} will only be called if and only if
+ * this method is called and returns false.
*
- * @return <code>true</code> if data can be read without blocking, else
- * <code>false</code>
+ * @return {@code true} if data can be obtained without blocking, otherwise
+ * returns {@code false}.
*
* @since Servlet 3.1
*/
@@ -118,4 +213,40 @@ public abstract class ServletInputStream extends InputStream {
* @since Servlet 3.1
*/
public abstract void setReadListener(ReadListener listener);
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * This method may only be used when the input stream is in blocking mode.
+ *
+ * @exception IllegalStateException If this method is called when the input stream is in non-blocking mode.
+ */
+ @Override
+ public byte[] readAllBytes() throws IOException {
+ return super.readAllBytes();
+ }
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * This method may only be used when the input stream is in blocking mode.
+ *
+ * @exception IllegalStateException If this method is called when the input stream is in non-blocking mode.
+ */
+ @Override
+ public byte[] readNBytes(int len) throws IOException {
+ return super.readNBytes(len);
+ }
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * This method may only be used when the input stream is in blocking mode.
+ *
+ * @exception IllegalStateException If this method is called when the input stream is in non-blocking mode.
+ */
+ @Override
+ public int readNBytes(byte[] b, int off, int len) throws IOException {
+ return super.readNBytes(b, off, len);
+ }
}
diff --git a/java/jakarta/servlet/ServletOutputStream.java b/java/jakarta/servlet/ServletOutputStream.java
index 86a57c6602..66ee5c1503 100644
--- a/java/jakarta/servlet/ServletOutputStream.java
+++ b/java/jakarta/servlet/ServletOutputStream.java
@@ -19,7 +19,9 @@ package jakarta.servlet;
import java.io.CharConversionException;
import java.io.IOException;
import java.io.OutputStream;
+import java.nio.ByteBuffer;
import java.text.MessageFormat;
+import java.util.Objects;
import java.util.ResourceBundle;
/**
@@ -45,6 +47,69 @@ public abstract class ServletOutputStream extends OutputStream {
// NOOP
}
+ /**
+ * Writes from the given buffer to the output stream.
+ * <p>
+ * If the output steam is in non-blocking mode, before each invocation of
+ * this method {@link #isReady()} must be called and must return
+ * {@code true} or the {@link WriteListener#onWritePossible()} call back
+ * must indicate that data may be written else an
+ * {@link IllegalStateException} must be thrown.
+ * <p>
+ * Otherwise, if this method is called when {@code buffer} has no data
+ * remaining, the method returns immediately and {@code buffer} is
+ * unchanged.
+ * <p>
+ * If the output stream is in non-blocking mode, neither the position, limit
+ * nor content of the buffer passed to this method may be modified until a
+ * subsequent call to {@link #isReady()} returns true or the
+ * {@link WriteListener#onWritePossible()} call back indicates data may be
+ * written again. At this point the buffer's limit will be unchanged from
+ * the value when passed to this method and the position will be the same as
+ * the limit.
+ * <p>
+ * If the output stream is in blocking mode and {@code buffer} has space
+ * remaining, this method blocks until all the remaining data in the buffer
+ * has been written. When the method returns, and if data has been written,
+ * the buffer's limit will be unchanged from the value when passed to this
+ * method and the position will be the same as the limit.
+ * <p>
+ * Subclasses are strongly encouraged to override this method and provide a
+ * more efficient implementation.
+ *
+ * @param buffer The buffer from which the data is written.
+ *
+ * @exception IllegalStateException If the output stream is in non-blocking
+ * mode and this method is called without first calling {@link #isReady()}
+ * and that method has returned {@code true} or
+ * {@link WriteListener#onWritePossible()} has not signalled that data may
+ * be written.
+ *
+ * @exception IOException If the output stream has been closed or if some
+ * other I/O error occurs.
+ *
+ * @exception NullPointerException If buffer is null.
+ *
+ * @since Servlet 6.1
+ */
+ public void write(ByteBuffer buffer) throws IOException {
+ Objects.requireNonNull(buffer);
+
+ if (!isReady()) {
+ throw new IllegalStateException();
+ }
+
+ if (buffer.remaining() == 0) {
+ return;
+ }
+
+ byte[] b = new byte[buffer.remaining()];
+
+ buffer.get(b);
+
+ write(b);
+ }
+
/**
* Writes a <code>String</code> to the client, without a carriage
* return-line feed (CRLF) character at the end.
@@ -278,13 +343,24 @@ public abstract class ServletOutputStream extends OutputStream {
}
/**
- * Checks if a non-blocking write will succeed. If this returns
- * <code>false</code>, it will cause a callback to
- * {@link WriteListener#onWritePossible()} when the buffer has emptied. If
- * this method returns <code>false</code> no further data must be written
- * until the container calls {@link WriteListener#onWritePossible()}.
+ * Returns {@code true} if it is allowable to call any method that may write
+ * data (e.g. {@code write()}, {@code print()} or {@code flush}). In
+ * blocking mode, this method will always return {@code true}, but a
+ * subsequent call to a method that writes data may block. In non-blocking
+ * mode this method may return {@code false}, in which case it is illegal to
+ * call a method that writes data and an {@link IllegalStateException} MUST
+ * be thrown. When {@link WriteListener#onWritePossible()} is called, a call
+ * to this method that returned {@code true} is implicit.
+ * <p>
+ * If this method returns {@code false} and a {@link WriteListener} has been
+ * set via {@link #setWriteListener(WriteListener)}, then container will
+ * subsequently invoke {@link WriteListener#onWritePossible()} once a write
+ * operation becomes possible without blocking. Other than the initial call,
+ * {@link WriteListener#onWritePossible()} will only be called if and only
+ * if this method is called and returns false.
*
- * @return <code>true</code> if data can be written, else <code>false</code>
+ * @return {@code true} if data can be written without blocking, otherwise
+ * returns {@code false}.
*
* @since Servlet 3.1
*/
@@ -306,4 +382,17 @@ public abstract class ServletOutputStream extends OutputStream {
* @since Servlet 3.1
*/
public abstract void setWriteListener(jakarta.servlet.WriteListener listener);
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * If this method is called when the output stream is in non-blocking mode, it will immediately return with the stream
+ * effectively closed, even if the stream contains buffered data that is yet to be written to client. The container will
+ * write this data out in the background. If this process fails the {@link WriteListener#onError(Throwable)} method will
+ * be invoked as normal.
+ */
+ @Override
+ public void close() throws IOException {
+ super.close();
+ }
}
diff --git a/java/org/apache/catalina/connector/CoyoteInputStream.java b/java/org/apache/catalina/connector/CoyoteInputStream.java
index ac3ce2d13c..01cc429903 100644
--- a/java/org/apache/catalina/connector/CoyoteInputStream.java
+++ b/java/org/apache/catalina/connector/CoyoteInputStream.java
@@ -21,6 +21,7 @@ import java.nio.ByteBuffer;
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
+import java.util.Objects;
import jakarta.servlet.ReadListener;
import jakarta.servlet.ServletInputStream;
@@ -134,18 +135,9 @@ public class CoyoteInputStream extends ServletInputStream {
}
- /**
- * Transfers bytes from the buffer to the specified ByteBuffer. After the
- * operation the position of the ByteBuffer will be returned to the one
- * before the operation, the limit will be the position incremented by
- * the number of the transferred bytes.
- *
- * @param b the ByteBuffer into which bytes are to be written.
- * @return an integer specifying the actual number of bytes read, or -1 if
- * the end of the stream is reached
- * @throws IOException if an input or output exception has occurred
- */
+ @Override
public int read(final ByteBuffer b) throws IOException {
+ Objects.requireNonNull(b);
checkNonBlockingRead();
if (SecurityUtil.isPackageProtectionEnabled()) {
diff --git a/java/org/apache/catalina/connector/CoyoteOutputStream.java b/java/org/apache/catalina/connector/CoyoteOutputStream.java
index ee005387b3..218fb00aaf 100644
--- a/java/org/apache/catalina/connector/CoyoteOutputStream.java
+++ b/java/org/apache/catalina/connector/CoyoteOutputStream.java
@@ -18,6 +18,7 @@ package org.apache.catalina.connector;
import java.io.IOException;
import java.nio.ByteBuffer;
+import java.util.Objects;
import jakarta.servlet.ServletOutputStream;
import jakarta.servlet.WriteListener;
@@ -100,8 +101,13 @@ public class CoyoteOutputStream extends ServletOutputStream {
}
+ @Override
public void write(ByteBuffer from) throws IOException {
+ Objects.requireNonNull(from);
boolean nonBlocking = checkNonBlockingWrite();
+ if (from.remaining() == 0) {
+ return;
+ }
ob.write(from);
if (nonBlocking) {
checkRegisterForWrite();
diff --git a/java/org/apache/catalina/connector/InputBuffer.java b/java/org/apache/catalina/connector/InputBuffer.java
index ac1b816207..8d1f5f97c5 100644
--- a/java/org/apache/catalina/connector/InputBuffer.java
+++ b/java/org/apache/catalina/connector/InputBuffer.java
@@ -359,6 +359,10 @@ public class InputBuffer extends Reader
public int read(ByteBuffer to) throws IOException {
throwIfClosed();
+ if (to.remaining() == 0) {
+ return 0;
+ }
+
if (checkByteBufferEof()) {
return -1;
}
diff --git a/java/org/apache/catalina/connector/LocalStrings.properties b/java/org/apache/catalina/connector/LocalStrings.properties
index 6726b4a303..09a1ed58f1 100644
--- a/java/org/apache/catalina/connector/LocalStrings.properties
+++ b/java/org/apache/catalina/connector/LocalStrings.properties
@@ -37,6 +37,7 @@ coyoteConnector.protocolHandlerStopFailed=Protocol handler stop failed
coyoteInputStream.nbNotready=In non-blocking mode you may not read from the ServletInputStream until the previous read has completed and isReady() returns true
coyoteInputStream.null=The input buffer object has been recycled and is no longer associated with this facade
+coyoteInputStream.blockingOnly=This method may not be called when the input stream is in non-blocking mode (i.e. after a ReadListener has been configured)
coyoteOutputStream.nbNotready=In non-blocking mode you may not write to the ServletOutputStream until the previous write has completed and isReady() returns true
coyoteOutputStream.null=The output buffer object has been recycled and is no longer associated with this facade
diff --git a/java/org/apache/coyote/LocalStrings.properties b/java/org/apache/coyote/LocalStrings.properties
index b708730f45..e06ffcd9a1 100644
--- a/java/org/apache/coyote/LocalStrings.properties
+++ b/java/org/apache/coyote/LocalStrings.properties
@@ -65,6 +65,5 @@ request.readListenerSet=The non-blocking read listener has already been set
response.encoding.invalid=The encoding [{0}] is not recognised by the JRE
response.noTrailers.notSupported=A trailer fields supplier may not be set for this response. Either the underlying protocol does not support trailer fields or the protocol requires that the supplier is set before the response is committed
response.notAsync=It is only valid to switch to non-blocking IO within async processing or HTTP upgrade processing
-response.notNonBlocking=It is invalid to call isReady() when the response has not been put into non-blocking mode
response.nullWriteListener=The listener passed to setWriteListener() may not be null
response.writeListenerSet=The non-blocking write listener has already been set
diff --git a/java/org/apache/coyote/Response.java b/java/org/apache/coyote/Response.java
index b3bd46022c..fadc3fc1c1 100644
--- a/java/org/apache/coyote/Response.java
+++ b/java/org/apache/coyote/Response.java
@@ -746,10 +746,7 @@ public final class Response {
public boolean isReady() {
if (listener == null) {
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("response.notNonBlocking"));
- }
- return false;
+ return true;
}
// Assume write is not possible
boolean ready = false;
diff --git a/java/org/apache/coyote/http11/Http11OutputBuffer.java b/java/org/apache/coyote/http11/Http11OutputBuffer.java
index cf666b605d..570b90c0d7 100644
--- a/java/org/apache/coyote/http11/Http11OutputBuffer.java
+++ b/java/org/apache/coyote/http11/Http11OutputBuffer.java
@@ -565,6 +565,13 @@ public class Http11OutputBuffer implements HttpOutputBuffer {
@Override
public void end() throws IOException {
+ /*
+ * TODO
+ * As of Servlet 6.1, this flush is (currently) meant to be
+ * non-blocking if the output stream is in non-blocking mode. That
+ * requirement creates various complications I want to discuss with
+ * the EG before I try implementing it.
+ */
socketWrapper.flush(true);
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 02755378c3..854f21011d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -105,6 +105,17 @@
issues do not "pop up" wrt. others).
-->
<section name="Tomcat 11.0.0-M2 (markt)" rtext="in development">
+ <subsection name="Catalina">
+ <changelog>
+ <add>
+ Update the <code>ServletInputStream</code> and
+ <code>ServletOuputStream</code> classes in the Servlet API to align with
+ the recent updates in the Jakarta Servlet specification to support
+ reading and writing with <code>ByteBuffer</code>s. The changes also
+ clarified various aspects of the Servlet non-blocking API. (markt)
+ </add>
+ </changelog>
+ </subsection>
<subsection name="Coyote">
<changelog>
<fix>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch main updated: Add support for Servlet read/write via ByteBuffer
Posted by Mark Thomas <ma...@apache.org>.
On 14/12/2022 11:21, Rémy Maucherat wrote:
> On Mon, Dec 12, 2022 at 4:43 PM <ma...@apache.org> wrote:
<snip/>
>> + /**
>> + * {@inheritDoc}
>> + * <p>
>> + * If this method is called when the output stream is in non-blocking mode, it will immediately return with the stream
>> + * effectively closed, even if the stream contains buffered data that is yet to be written to client. The container will
>> + * write this data out in the background. If this process fails the {@link WriteListener#onError(Throwable)} method will
>> + * be invoked as normal.
>> + */
>> + @Override
>> + public void close() throws IOException {
>> + super.close();
>> + }
<snip/>
>> diff --git a/java/org/apache/coyote/http11/Http11OutputBuffer.java b/java/org/apache/coyote/http11/Http11OutputBuffer.java
>> index cf666b605d..570b90c0d7 100644
>> --- a/java/org/apache/coyote/http11/Http11OutputBuffer.java
>> +++ b/java/org/apache/coyote/http11/Http11OutputBuffer.java
>> @@ -565,6 +565,13 @@ public class Http11OutputBuffer implements HttpOutputBuffer {
>>
>> @Override
>> public void end() throws IOException {
>> + /*
>> + * TODO
>> + * As of Servlet 6.1, this flush is (currently) meant to be
>> + * non-blocking if the output stream is in non-blocking mode. That
>> + * requirement creates various complications I want to discuss with
>> + * the EG before I try implementing it.
>> + */
>> socketWrapper.flush(true);
>> }
>
> Ok there are the main points:
> - Flushing shouldn't be used at all in non blocking, since data will
> always be sent ASAP. So if you flush in the app, then it will have to
> break the API promise (not blocking). Overall, if flush is allowed to
> be called in non blocking, then it should be a noop instead since it
> will never do anything useful (if the app wants to know when writing
> is done, then it should wait for the notification).
> - If (I think that's the case here) the app server blocks to wait for
> the end of the request without anything from the app on the call
> stack, then it possibly degrades scalability a bit but it's not
> actually going to be a problem in the real world.
That comment is there due to the clarification added to
ServletOutputStream.close().
I have a patch that might work for the clarified close behaviour but I
need to find some time to write some test cases to check it. If it
doesn't work out, the current behaviour seems reasonable to me although
I'll probably go back tot he EG to discuss options.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch main updated: Add support for Servlet read/write via ByteBuffer
Posted by Rémy Maucherat <re...@apache.org>.
On Mon, Dec 12, 2022 at 4:43 PM <ma...@apache.org> wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch main
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/main by this push:
> new 2d931eed6c Add support for Servlet read/write via ByteBuffer
> 2d931eed6c is described below
>
> commit 2d931eed6c35702ef6541c0fd6c067b79e5eb371
> Author: Mark Thomas <ma...@apache.org>
> AuthorDate: Mon Dec 12 15:43:28 2022 +0000
>
> Add support for Servlet read/write via ByteBuffer
> ---
> java/jakarta/servlet/ServletInputStream.java | 141 ++++++++++++++++++++-
> java/jakarta/servlet/ServletOutputStream.java | 101 ++++++++++++++-
> .../catalina/connector/CoyoteInputStream.java | 14 +-
> .../catalina/connector/CoyoteOutputStream.java | 6 +
> .../org/apache/catalina/connector/InputBuffer.java | 4 +
> .../catalina/connector/LocalStrings.properties | 1 +
> java/org/apache/coyote/LocalStrings.properties | 1 -
> java/org/apache/coyote/Response.java | 5 +-
> .../apache/coyote/http11/Http11OutputBuffer.java | 7 +
> webapps/docs/changelog.xml | 11 ++
> 10 files changed, 264 insertions(+), 27 deletions(-)
>
> diff --git a/java/jakarta/servlet/ServletInputStream.java b/java/jakarta/servlet/ServletInputStream.java
> index 0c8280c2e9..10048fbb88 100644
> --- a/java/jakarta/servlet/ServletInputStream.java
> +++ b/java/jakarta/servlet/ServletInputStream.java
> @@ -18,6 +18,8 @@ package jakarta.servlet;
>
> import java.io.IOException;
> import java.io.InputStream;
> +import java.nio.ByteBuffer;
> +import java.util.Objects;
>
> /**
> * Provides an input stream for reading binary data from a client request,
> @@ -43,6 +45,80 @@ public abstract class ServletInputStream extends InputStream {
> // NOOP
> }
>
> + /**
> + * Reads from the input stream into the given buffer.
> + * <p>
> + * If the input stream is in non-blocking mode, before each invocation of
> + * this method {@link #isReady()} must be called and must return
> + * {@code true} or the {@link ReadListener#onDataAvailable()} call back must
> + * indicate that data is available to read else an
> + * {@link IllegalStateException} must be thrown.
> + * <p>
> + * Otherwise, if this method is called when {@code buffer} has no space
> + * remaining, the method returns {@code 0} immediately and {@code buffer} is
> + * unchanged.
> + * <p>
> + * If the input stream is in blocking mode and {@code buffer} has space
> + * remaining, this method blocks until at least one byte has been read, end
> + * of stream is reached or an exception is thrown.
> + * <p>
> + * Returns the number of bytes read or {@code -1} if the end of stream is
> + * reached without reading any data.
> + * <p>
> + * When the method returns, and if data has been read, the buffer's position
> + * will be unchanged from the value when passed to this method and the limit
> + * will be the position incremented by the number of bytes read.
> + * <p>
> + * Subclasses are strongly encouraged to override this method and provide a
> + * more efficient implementation.
> + *
> + * @param buffer The buffer into which the data is read.
> + *
> + * @return The number of bytes read or {@code -1} if the end of the stream
> + * has been reached.
> + *
> + * @exception IllegalStateException If the input stream is in non-blocking
> + * mode and this method is called without first calling {@link #isReady()}
> + * and that method has returned {@code true} or
> + * {@link ReadListener#onDataAvailable()} has not signalled that data is
> + * available to read.
> + *
> + * @exception IOException If data cannot be read for any reason other than
> + * the end of stream being reached, the input stream has been closed or if
> + * some other I/O error occurs.
> + *
> + * @exception NullPointerException If buffer is null.
> + *
> + * @since Servlet 6.1
> + */
> + public int read(ByteBuffer buffer) throws IOException {
> + Objects.requireNonNull(buffer);
> +
> + if (!isReady()) {
> + throw new IllegalStateException();
> + }
> +
> + if (buffer.remaining() == 0) {
> + return 0;
> + }
> +
> + byte[] b = new byte[buffer.remaining()];
> +
> + int result = read(b);
> + if (result == -1) {
> + return -1;
> + }
> +
> + int position = buffer.position();
> +
> + buffer.put(b, 0, result);
> +
> + buffer.position(position);
> + buffer.limit(position + result);
> +
> + return result;
> + }
> +
> /**
> * Reads the input stream, one line at a time. Starting at an offset, reads
> * bytes into an array, until it reads a certain number of bytes or reaches
> @@ -50,6 +126,8 @@ public abstract class ServletInputStream extends InputStream {
> * <p>
> * This method returns -1 if it reaches the end of the input stream before
> * reading the maximum number of bytes.
> + * <p>
> + * This method may only be used when the input stream is in blocking mode.
> *
> * @param b
> * an array of bytes into which data is read
> @@ -60,6 +138,10 @@ public abstract class ServletInputStream extends InputStream {
> * an integer specifying the maximum number of bytes to read
> * @return an integer specifying the actual number of bytes read, or -1 if
> * the end of the stream is reached
> + *
> + * @exception IllegalStateException
> + * If this method is called when the input stream is in
> + * non-blocking mode.
> * @exception IOException
> * if an input or output exception has occurred
> */
> @@ -91,12 +173,25 @@ public abstract class ServletInputStream extends InputStream {
> public abstract boolean isFinished();
>
> /**
> - * Can data be read from this InputStream without blocking?
> - * Returns If this method is called and returns false, the container will
> - * invoke {@link ReadListener#onDataAvailable()} when data is available.
> + * Returns {@code true} if it is allowable to call a {@code read()} method.
> + * In blocking mode, this method will always return {@code true}, but a
> + * subsequent call to a {@code read()} method may block awaiting data. In
> + * non-blocking mode this method may return {@code false}, in which case it
> + * is illegal to call a {@code read()} method and an
> + * {@link IllegalStateException} MUST be thrown. When
> + * {@link ReadListener#onDataAvailable()} is called, a call to this method
> + * that returned {@code true} is implicit.
> + * <p>
> + * If this method returns {@code false} and a {@link ReadListener} has been
> + * set via {@link #setReadListener(ReadListener)}, then the container will
> + * subsequently invoke {@link ReadListener#onDataAvailable()} (or
> + * {@link ReadListener#onAllDataRead()}) once data (or EOF) has become
> + * available. Other than the initial call
> + * {@link ReadListener#onDataAvailable()} will only be called if and only if
> + * this method is called and returns false.
> *
> - * @return <code>true</code> if data can be read without blocking, else
> - * <code>false</code>
> + * @return {@code true} if data can be obtained without blocking, otherwise
> + * returns {@code false}.
> *
> * @since Servlet 3.1
> */
> @@ -118,4 +213,40 @@ public abstract class ServletInputStream extends InputStream {
> * @since Servlet 3.1
> */
> public abstract void setReadListener(ReadListener listener);
> +
> + /**
> + * {@inheritDoc}
> + * <p>
> + * This method may only be used when the input stream is in blocking mode.
> + *
> + * @exception IllegalStateException If this method is called when the input stream is in non-blocking mode.
> + */
> + @Override
> + public byte[] readAllBytes() throws IOException {
> + return super.readAllBytes();
> + }
> +
> + /**
> + * {@inheritDoc}
> + * <p>
> + * This method may only be used when the input stream is in blocking mode.
> + *
> + * @exception IllegalStateException If this method is called when the input stream is in non-blocking mode.
> + */
> + @Override
> + public byte[] readNBytes(int len) throws IOException {
> + return super.readNBytes(len);
> + }
> +
> + /**
> + * {@inheritDoc}
> + * <p>
> + * This method may only be used when the input stream is in blocking mode.
> + *
> + * @exception IllegalStateException If this method is called when the input stream is in non-blocking mode.
> + */
> + @Override
> + public int readNBytes(byte[] b, int off, int len) throws IOException {
> + return super.readNBytes(b, off, len);
> + }
> }
> diff --git a/java/jakarta/servlet/ServletOutputStream.java b/java/jakarta/servlet/ServletOutputStream.java
> index 86a57c6602..66ee5c1503 100644
> --- a/java/jakarta/servlet/ServletOutputStream.java
> +++ b/java/jakarta/servlet/ServletOutputStream.java
> @@ -19,7 +19,9 @@ package jakarta.servlet;
> import java.io.CharConversionException;
> import java.io.IOException;
> import java.io.OutputStream;
> +import java.nio.ByteBuffer;
> import java.text.MessageFormat;
> +import java.util.Objects;
> import java.util.ResourceBundle;
>
> /**
> @@ -45,6 +47,69 @@ public abstract class ServletOutputStream extends OutputStream {
> // NOOP
> }
>
> + /**
> + * Writes from the given buffer to the output stream.
> + * <p>
> + * If the output steam is in non-blocking mode, before each invocation of
> + * this method {@link #isReady()} must be called and must return
> + * {@code true} or the {@link WriteListener#onWritePossible()} call back
> + * must indicate that data may be written else an
> + * {@link IllegalStateException} must be thrown.
> + * <p>
> + * Otherwise, if this method is called when {@code buffer} has no data
> + * remaining, the method returns immediately and {@code buffer} is
> + * unchanged.
> + * <p>
> + * If the output stream is in non-blocking mode, neither the position, limit
> + * nor content of the buffer passed to this method may be modified until a
> + * subsequent call to {@link #isReady()} returns true or the
> + * {@link WriteListener#onWritePossible()} call back indicates data may be
> + * written again. At this point the buffer's limit will be unchanged from
> + * the value when passed to this method and the position will be the same as
> + * the limit.
> + * <p>
> + * If the output stream is in blocking mode and {@code buffer} has space
> + * remaining, this method blocks until all the remaining data in the buffer
> + * has been written. When the method returns, and if data has been written,
> + * the buffer's limit will be unchanged from the value when passed to this
> + * method and the position will be the same as the limit.
> + * <p>
> + * Subclasses are strongly encouraged to override this method and provide a
> + * more efficient implementation.
> + *
> + * @param buffer The buffer from which the data is written.
> + *
> + * @exception IllegalStateException If the output stream is in non-blocking
> + * mode and this method is called without first calling {@link #isReady()}
> + * and that method has returned {@code true} or
> + * {@link WriteListener#onWritePossible()} has not signalled that data may
> + * be written.
> + *
> + * @exception IOException If the output stream has been closed or if some
> + * other I/O error occurs.
> + *
> + * @exception NullPointerException If buffer is null.
> + *
> + * @since Servlet 6.1
> + */
> + public void write(ByteBuffer buffer) throws IOException {
> + Objects.requireNonNull(buffer);
> +
> + if (!isReady()) {
> + throw new IllegalStateException();
> + }
> +
> + if (buffer.remaining() == 0) {
> + return;
> + }
> +
> + byte[] b = new byte[buffer.remaining()];
> +
> + buffer.get(b);
> +
> + write(b);
> + }
> +
> /**
> * Writes a <code>String</code> to the client, without a carriage
> * return-line feed (CRLF) character at the end.
> @@ -278,13 +343,24 @@ public abstract class ServletOutputStream extends OutputStream {
> }
>
> /**
> - * Checks if a non-blocking write will succeed. If this returns
> - * <code>false</code>, it will cause a callback to
> - * {@link WriteListener#onWritePossible()} when the buffer has emptied. If
> - * this method returns <code>false</code> no further data must be written
> - * until the container calls {@link WriteListener#onWritePossible()}.
> + * Returns {@code true} if it is allowable to call any method that may write
> + * data (e.g. {@code write()}, {@code print()} or {@code flush}). In
> + * blocking mode, this method will always return {@code true}, but a
> + * subsequent call to a method that writes data may block. In non-blocking
> + * mode this method may return {@code false}, in which case it is illegal to
> + * call a method that writes data and an {@link IllegalStateException} MUST
> + * be thrown. When {@link WriteListener#onWritePossible()} is called, a call
> + * to this method that returned {@code true} is implicit.
> + * <p>
> + * If this method returns {@code false} and a {@link WriteListener} has been
> + * set via {@link #setWriteListener(WriteListener)}, then container will
> + * subsequently invoke {@link WriteListener#onWritePossible()} once a write
> + * operation becomes possible without blocking. Other than the initial call,
> + * {@link WriteListener#onWritePossible()} will only be called if and only
> + * if this method is called and returns false.
> *
> - * @return <code>true</code> if data can be written, else <code>false</code>
> + * @return {@code true} if data can be written without blocking, otherwise
> + * returns {@code false}.
> *
> * @since Servlet 3.1
> */
> @@ -306,4 +382,17 @@ public abstract class ServletOutputStream extends OutputStream {
> * @since Servlet 3.1
> */
> public abstract void setWriteListener(jakarta.servlet.WriteListener listener);
> +
> + /**
> + * {@inheritDoc}
> + * <p>
> + * If this method is called when the output stream is in non-blocking mode, it will immediately return with the stream
> + * effectively closed, even if the stream contains buffered data that is yet to be written to client. The container will
> + * write this data out in the background. If this process fails the {@link WriteListener#onError(Throwable)} method will
> + * be invoked as normal.
> + */
> + @Override
> + public void close() throws IOException {
> + super.close();
> + }
> }
> diff --git a/java/org/apache/catalina/connector/CoyoteInputStream.java b/java/org/apache/catalina/connector/CoyoteInputStream.java
> index ac3ce2d13c..01cc429903 100644
> --- a/java/org/apache/catalina/connector/CoyoteInputStream.java
> +++ b/java/org/apache/catalina/connector/CoyoteInputStream.java
> @@ -21,6 +21,7 @@ import java.nio.ByteBuffer;
> import java.security.AccessController;
> import java.security.PrivilegedActionException;
> import java.security.PrivilegedExceptionAction;
> +import java.util.Objects;
>
> import jakarta.servlet.ReadListener;
> import jakarta.servlet.ServletInputStream;
> @@ -134,18 +135,9 @@ public class CoyoteInputStream extends ServletInputStream {
> }
>
>
> - /**
> - * Transfers bytes from the buffer to the specified ByteBuffer. After the
> - * operation the position of the ByteBuffer will be returned to the one
> - * before the operation, the limit will be the position incremented by
> - * the number of the transferred bytes.
> - *
> - * @param b the ByteBuffer into which bytes are to be written.
> - * @return an integer specifying the actual number of bytes read, or -1 if
> - * the end of the stream is reached
> - * @throws IOException if an input or output exception has occurred
> - */
> + @Override
> public int read(final ByteBuffer b) throws IOException {
> + Objects.requireNonNull(b);
> checkNonBlockingRead();
>
> if (SecurityUtil.isPackageProtectionEnabled()) {
> diff --git a/java/org/apache/catalina/connector/CoyoteOutputStream.java b/java/org/apache/catalina/connector/CoyoteOutputStream.java
> index ee005387b3..218fb00aaf 100644
> --- a/java/org/apache/catalina/connector/CoyoteOutputStream.java
> +++ b/java/org/apache/catalina/connector/CoyoteOutputStream.java
> @@ -18,6 +18,7 @@ package org.apache.catalina.connector;
>
> import java.io.IOException;
> import java.nio.ByteBuffer;
> +import java.util.Objects;
>
> import jakarta.servlet.ServletOutputStream;
> import jakarta.servlet.WriteListener;
> @@ -100,8 +101,13 @@ public class CoyoteOutputStream extends ServletOutputStream {
> }
>
>
> + @Override
> public void write(ByteBuffer from) throws IOException {
> + Objects.requireNonNull(from);
> boolean nonBlocking = checkNonBlockingWrite();
> + if (from.remaining() == 0) {
> + return;
> + }
> ob.write(from);
> if (nonBlocking) {
> checkRegisterForWrite();
> diff --git a/java/org/apache/catalina/connector/InputBuffer.java b/java/org/apache/catalina/connector/InputBuffer.java
> index ac1b816207..8d1f5f97c5 100644
> --- a/java/org/apache/catalina/connector/InputBuffer.java
> +++ b/java/org/apache/catalina/connector/InputBuffer.java
> @@ -359,6 +359,10 @@ public class InputBuffer extends Reader
> public int read(ByteBuffer to) throws IOException {
> throwIfClosed();
>
> + if (to.remaining() == 0) {
> + return 0;
> + }
> +
> if (checkByteBufferEof()) {
> return -1;
> }
> diff --git a/java/org/apache/catalina/connector/LocalStrings.properties b/java/org/apache/catalina/connector/LocalStrings.properties
> index 6726b4a303..09a1ed58f1 100644
> --- a/java/org/apache/catalina/connector/LocalStrings.properties
> +++ b/java/org/apache/catalina/connector/LocalStrings.properties
> @@ -37,6 +37,7 @@ coyoteConnector.protocolHandlerStopFailed=Protocol handler stop failed
> coyoteInputStream.nbNotready=In non-blocking mode you may not read from the ServletInputStream until the previous read has completed and isReady() returns true
> coyoteInputStream.null=The input buffer object has been recycled and is no longer associated with this facade
>
> +coyoteInputStream.blockingOnly=This method may not be called when the input stream is in non-blocking mode (i.e. after a ReadListener has been configured)
> coyoteOutputStream.nbNotready=In non-blocking mode you may not write to the ServletOutputStream until the previous write has completed and isReady() returns true
> coyoteOutputStream.null=The output buffer object has been recycled and is no longer associated with this facade
>
> diff --git a/java/org/apache/coyote/LocalStrings.properties b/java/org/apache/coyote/LocalStrings.properties
> index b708730f45..e06ffcd9a1 100644
> --- a/java/org/apache/coyote/LocalStrings.properties
> +++ b/java/org/apache/coyote/LocalStrings.properties
> @@ -65,6 +65,5 @@ request.readListenerSet=The non-blocking read listener has already been set
> response.encoding.invalid=The encoding [{0}] is not recognised by the JRE
> response.noTrailers.notSupported=A trailer fields supplier may not be set for this response. Either the underlying protocol does not support trailer fields or the protocol requires that the supplier is set before the response is committed
> response.notAsync=It is only valid to switch to non-blocking IO within async processing or HTTP upgrade processing
> -response.notNonBlocking=It is invalid to call isReady() when the response has not been put into non-blocking mode
> response.nullWriteListener=The listener passed to setWriteListener() may not be null
> response.writeListenerSet=The non-blocking write listener has already been set
> diff --git a/java/org/apache/coyote/Response.java b/java/org/apache/coyote/Response.java
> index b3bd46022c..fadc3fc1c1 100644
> --- a/java/org/apache/coyote/Response.java
> +++ b/java/org/apache/coyote/Response.java
> @@ -746,10 +746,7 @@ public final class Response {
>
> public boolean isReady() {
> if (listener == null) {
> - if (log.isDebugEnabled()) {
> - log.debug(sm.getString("response.notNonBlocking"));
> - }
> - return false;
> + return true;
> }
> // Assume write is not possible
> boolean ready = false;
Yeah, it's probably good to flip this, but it's not really supposed to
be used in the first place.
> diff --git a/java/org/apache/coyote/http11/Http11OutputBuffer.java b/java/org/apache/coyote/http11/Http11OutputBuffer.java
> index cf666b605d..570b90c0d7 100644
> --- a/java/org/apache/coyote/http11/Http11OutputBuffer.java
> +++ b/java/org/apache/coyote/http11/Http11OutputBuffer.java
> @@ -565,6 +565,13 @@ public class Http11OutputBuffer implements HttpOutputBuffer {
>
> @Override
> public void end() throws IOException {
> + /*
> + * TODO
> + * As of Servlet 6.1, this flush is (currently) meant to be
> + * non-blocking if the output stream is in non-blocking mode. That
> + * requirement creates various complications I want to discuss with
> + * the EG before I try implementing it.
> + */
> socketWrapper.flush(true);
> }
Ok there are the main points:
- Flushing shouldn't be used at all in non blocking, since data will
always be sent ASAP. So if you flush in the app, then it will have to
break the API promise (not blocking). Overall, if flush is allowed to
be called in non blocking, then it should be a noop instead since it
will never do anything useful (if the app wants to know when writing
is done, then it should wait for the notification).
- If (I think that's the case here) the app server blocks to wait for
the end of the request without anything from the app on the call
stack, then it possibly degrades scalability a bit but it's not
actually going to be a problem in the real world.
Remy
>
> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> index 02755378c3..854f21011d 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -105,6 +105,17 @@
> issues do not "pop up" wrt. others).
> -->
> <section name="Tomcat 11.0.0-M2 (markt)" rtext="in development">
> + <subsection name="Catalina">
> + <changelog>
> + <add>
> + Update the <code>ServletInputStream</code> and
> + <code>ServletOuputStream</code> classes in the Servlet API to align with
> + the recent updates in the Jakarta Servlet specification to support
> + reading and writing with <code>ByteBuffer</code>s. The changes also
> + clarified various aspects of the Servlet non-blocking API. (markt)
> + </add>
> + </changelog>
> + </subsection>
> <subsection name="Coyote">
> <changelog>
> <fix>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org