On 06/06/2019 22:19, Rémy Maucherat wrote:
> On Thu, Jun 6, 2019 at 10:46 PM <[email protected]
> <mailto:[email protected]>> wrote:
>
> 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 e967bbc7f106f8ec5384a294d98d5cbc44b474f1
> Author: Mark Thomas <[email protected] <mailto:[email protected]>>
> AuthorDate: Thu Jun 6 21:45:30 2019 +0100
>
> Remove a source of potential deadlocks
>
> SocketWrapperBase.vectoredOperation acquires the (read|write)
> Semaphore
> and then locks the SocketWrapper during completion before the
> Semaphore
> is release. If the SocketWrapper is locked on some, but not all,
> paths
> before acquiring the Semaphore, a deadlock becomes possible.
>
> Fixed by removing the initial lock on SocketWrapper since the
> Semaphores
> make it unnecessary.
>
>
> I wished it worked, as this sync lowers performance a lot, but
> unfortunately it doesn't. The lock ensures that HPACK behaves and that
> everything gets written in order, more or less.
> Details and test is in https://bz.apache.org/bugzilla/show_bug.cgi?id=61740
> It fails again after removing the sync.
Thanks for the pointer.
This needs some more thought since with the sync you get deadlocks.
I haven't quite tracked down the other deadlock yet. The timing gap is
narrower so it is harder to capture. I have some ideas to work on tomorrow.
I think it would be useful to ID the other deadlock before thinking too
hard about how to solve the problem above as it may introduce additional
complications.
Mark
>
> Rémy
>
>
> ---
> .../apache/coyote/http2/Http2AsyncUpgradeHandler.java | 17
> +++++++----------
> webapps/docs/changelog.xml | 9 +++++++++
> 2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git
> a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
> b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
> index 969bfda..8a57c53 100644
> --- a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
> +++ b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
> @@ -169,16 +169,13 @@ public class Http2AsyncUpgradeHandler extends
> Http2UpgradeHandler {
> @Override
> void writeHeaders(Stream stream, int pushedStreamId,
> MimeHeaders mimeHeaders,
> boolean endOfStream, int payloadSize) throws IOException {
> - // This ensures the Stream processing thread has control of
> the socket.
> - synchronized (socketWrapper) {
> - AsyncHeaderFrameBuffers headerFrameBuffers =
> (AsyncHeaderFrameBuffers)
> - doWriteHeaders(stream, pushedStreamId,
> mimeHeaders, endOfStream, payloadSize);
> - if (headerFrameBuffers != null) {
> - socketWrapper.write(BlockingMode.SEMI_BLOCK,
> protocol.getWriteTimeout(),
> - TimeUnit.MILLISECONDS, null,
> SocketWrapperBase.COMPLETE_WRITE,
> - applicationErrorCompletion,
> headerFrameBuffers.bufs.toArray(BYTEBUFFER_ARRAY));
> - handleAsyncException();
> - }
> + AsyncHeaderFrameBuffers headerFrameBuffers =
> (AsyncHeaderFrameBuffers)
> + doWriteHeaders(stream, pushedStreamId, mimeHeaders,
> endOfStream, payloadSize);
> + if (headerFrameBuffers != null) {
> + socketWrapper.write(BlockingMode.SEMI_BLOCK,
> protocol.getWriteTimeout(),
> + TimeUnit.MILLISECONDS, null,
> SocketWrapperBase.COMPLETE_WRITE,
> + applicationErrorCompletion,
> headerFrameBuffers.bufs.toArray(BYTEBUFFER_ARRAY));
> + handleAsyncException();
> }
> if (endOfStream) {
> stream.sentEndOfStream();
> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> index 7f55e3d..d65a1f3 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -45,6 +45,15 @@
> issues do not "pop up" wrt. others).
> -->
> <section name="Tomcat 9.0.22 (markt)" rtext="in development">
> + <subsection name="Coyote">
> + <changelog>
> + <fix>
> + Remove a source of potential deadlocks when using HTTP/2
> when the
> + Connector is configured with <code>useAsyncIO</code> as
> + <code>true</code>. (markt)
> + </fix>
> + </changelog>
> + </subsection>
> </section>
> <section name="Tomcat 9.0.21 (markt)" rtext="release in progress">
> <subsection name="Catalina">
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> <mailto:[email protected]>
> For additional commands, e-mail: [email protected]
> <mailto:[email protected]>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
