You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2019/06/07 09:58:45 UTC

[httpcomponents-core] branch bug-fixes created (now f82290e)

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a change to branch bug-fixes
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git.


      at f82290e  Improved exception handling in the async HTTP protocol negotiators

This branch includes the following new commits:

     new f82290e  Improved exception handling in the async HTTP protocol negotiators

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[httpcomponents-core] 01/01: Improved exception handling in the async HTTP protocol negotiators

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch bug-fixes
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git

commit f82290ee67e2b6745b38e04f3097f02396fccffb
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Fri Jun 7 11:52:52 2019 +0200

    Improved exception handling in the async HTTP protocol negotiators
---
 .../impl/nio/ClientHttpProtocolNegotiator.java     | 71 ++++++++--------------
 .../impl/nio/ServerHttpProtocolNegotiator.java     | 32 +++++++---
 2 files changed, 47 insertions(+), 56 deletions(-)

diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientHttpProtocolNegotiator.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientHttpProtocolNegotiator.java
index 6b3a97e..3dc0fc1 100644
--- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientHttpProtocolNegotiator.java
+++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientHttpProtocolNegotiator.java
@@ -32,25 +32,21 @@ import java.net.SocketAddress;
 import java.nio.ByteBuffer;
 import java.nio.channels.ByteChannel;
 import java.nio.channels.SelectionKey;
+import java.util.concurrent.atomic.AtomicReference;
 
 import javax.net.ssl.SSLSession;
 
 import org.apache.hc.core5.annotation.Internal;
-import org.apache.hc.core5.http.ConnectionClosedException;
 import org.apache.hc.core5.http.EndpointDetails;
 import org.apache.hc.core5.http.ProtocolVersion;
 import org.apache.hc.core5.http.impl.nio.ClientHttp1IOEventHandler;
 import org.apache.hc.core5.http.impl.nio.ClientHttp1StreamDuplexer;
 import org.apache.hc.core5.http.impl.nio.ClientHttp1StreamDuplexerFactory;
 import org.apache.hc.core5.http.impl.nio.HttpConnectionEventHandler;
-import org.apache.hc.core5.http.nio.AsyncClientExchangeHandler;
-import org.apache.hc.core5.http.nio.command.RequestExecutionCommand;
 import org.apache.hc.core5.http2.HttpVersionPolicy;
 import org.apache.hc.core5.http2.ssl.ApplicationProtocols;
 import org.apache.hc.core5.io.CloseMode;
 import org.apache.hc.core5.io.SocketTimeoutExceptionFactory;
-import org.apache.hc.core5.reactor.Command;
-import org.apache.hc.core5.reactor.IOEventHandler;
 import org.apache.hc.core5.reactor.IOSession;
 import org.apache.hc.core5.reactor.ProtocolIOSession;
 import org.apache.hc.core5.reactor.ssl.TlsDetails;
@@ -77,6 +73,7 @@ public class ClientHttpProtocolNegotiator implements HttpConnectionEventHandler
     private final ClientHttp1StreamDuplexerFactory http1StreamHandlerFactory;
     private final ClientHttp2StreamMultiplexerFactory http2StreamHandlerFactory;
     private final HttpVersionPolicy versionPolicy;
+    private final AtomicReference<HttpConnectionEventHandler> protocolHandlerRef;
 
     private volatile ByteBuffer preface;
 
@@ -89,28 +86,31 @@ public class ClientHttpProtocolNegotiator implements HttpConnectionEventHandler
         this.http1StreamHandlerFactory = Args.notNull(http1StreamHandlerFactory, "HTTP/1.1 stream handler factory");
         this.http2StreamHandlerFactory = Args.notNull(http2StreamHandlerFactory, "HTTP/2 stream handler factory");
         this.versionPolicy = versionPolicy != null ? versionPolicy : HttpVersionPolicy.NEGOTIATE;
+        this.protocolHandlerRef = new AtomicReference<>(null);
     }
 
     private void startHttp1(final IOSession session) {
         final ClientHttp1StreamDuplexer http1StreamHandler = http1StreamHandlerFactory.create(ioSession);
-        final ClientHttp1IOEventHandler newHandler = new ClientHttp1IOEventHandler(http1StreamHandler);
+        final HttpConnectionEventHandler protocolHandler = new ClientHttp1IOEventHandler(http1StreamHandler);
         try {
-            ioSession.upgrade(newHandler);
-            newHandler.connected(session);
+            ioSession.upgrade(protocolHandler);
+            protocolHandlerRef.set(protocolHandler);
+            protocolHandler.connected(session);
         } catch (final Exception ex) {
-            newHandler.exception(session, ex);
+            protocolHandler.exception(session, ex);
             session.close(CloseMode.IMMEDIATE);
         }
     }
 
     private void startHttp2(final IOSession session) {
         final ClientHttp2StreamMultiplexer streamMultiplexer = http2StreamHandlerFactory.create(ioSession);
-        final IOEventHandler newHandler = new ClientHttp2IOEventHandler(streamMultiplexer);
+        final HttpConnectionEventHandler protocolHandler = new ClientHttp2IOEventHandler(streamMultiplexer);
         try {
-            ioSession.upgrade(newHandler);
-            newHandler.connected(session);
+            ioSession.upgrade(protocolHandler);
+            protocolHandlerRef.set(protocolHandler);
+            protocolHandler.connected(session);
         } catch (final Exception ex) {
-            newHandler.exception(session, ex);
+            protocolHandler.exception(session, ex);
             session.close(CloseMode.IMMEDIATE);
         }
     }
@@ -172,43 +172,18 @@ public class ClientHttpProtocolNegotiator implements HttpConnectionEventHandler
 
     @Override
     public void exception(final IOSession session, final Exception cause) {
-        try {
-            for (;;) {
-                final Command command = ioSession.poll();
-                if (command != null) {
-                    if (command instanceof RequestExecutionCommand) {
-                        final RequestExecutionCommand executionCommand = (RequestExecutionCommand) command;
-                        final AsyncClientExchangeHandler exchangeHandler = executionCommand.getExchangeHandler();
-                        exchangeHandler.failed(cause);
-                        exchangeHandler.releaseResources();
-                    } else {
-                        command.cancel();
-                    }
-                } else {
-                    break;
-                }
-            }
-        } finally {
-            session.close(CloseMode.IMMEDIATE);
+        final HttpConnectionEventHandler protocolHandler = protocolHandlerRef.get();
+        if (protocolHandler != null) {
+            protocolHandler.exception(session, cause);
         }
+        session.close(CloseMode.IMMEDIATE);
     }
 
     @Override
     public void disconnected(final IOSession session) {
-        for (;;) {
-            final Command command = ioSession.poll();
-            if (command != null) {
-                if (command instanceof RequestExecutionCommand) {
-                    final RequestExecutionCommand executionCommand = (RequestExecutionCommand) command;
-                    final AsyncClientExchangeHandler exchangeHandler = executionCommand.getExchangeHandler();
-                    exchangeHandler.failed(new ConnectionClosedException());
-                    exchangeHandler.releaseResources();
-                } else {
-                    command.cancel();
-                }
-            } else {
-                break;
-            }
+        final HttpConnectionEventHandler protocolHandler = protocolHandlerRef.getAndSet(null);
+        if (protocolHandler != null) {
+            protocolHandler.disconnected(ioSession);
         }
     }
 
@@ -220,7 +195,8 @@ public class ClientHttpProtocolNegotiator implements HttpConnectionEventHandler
 
     @Override
     public EndpointDetails getEndpointDetails() {
-        return null;
+        final HttpConnectionEventHandler protocolHandler = protocolHandlerRef.get();
+        return protocolHandler != null ? protocolHandler.getEndpointDetails() : null;
     }
 
     @Override
@@ -235,7 +211,8 @@ public class ClientHttpProtocolNegotiator implements HttpConnectionEventHandler
 
     @Override
     public ProtocolVersion getProtocolVersion() {
-        return null;
+        final HttpConnectionEventHandler protocolHandler = protocolHandlerRef.get();
+        return protocolHandler != null ? protocolHandler.getProtocolVersion() : null;
     }
 
     @Override
diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ServerHttpProtocolNegotiator.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ServerHttpProtocolNegotiator.java
index 165f10d..84d6714 100644
--- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ServerHttpProtocolNegotiator.java
+++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ServerHttpProtocolNegotiator.java
@@ -30,6 +30,7 @@ package org.apache.hc.core5.http2.impl.nio;
 import java.io.IOException;
 import java.net.SocketAddress;
 import java.nio.ByteBuffer;
+import java.util.concurrent.atomic.AtomicReference;
 
 import javax.net.ssl.SSLSession;
 
@@ -47,7 +48,6 @@ import org.apache.hc.core5.http2.HttpVersionPolicy;
 import org.apache.hc.core5.http2.ssl.ApplicationProtocols;
 import org.apache.hc.core5.io.CloseMode;
 import org.apache.hc.core5.io.SocketTimeoutExceptionFactory;
-import org.apache.hc.core5.reactor.IOEventHandler;
 import org.apache.hc.core5.reactor.IOSession;
 import org.apache.hc.core5.reactor.ProtocolIOSession;
 import org.apache.hc.core5.reactor.ssl.TlsDetails;
@@ -71,6 +71,7 @@ public class ServerHttpProtocolNegotiator implements HttpConnectionEventHandler
     private final ServerHttp2StreamMultiplexerFactory http2StreamHandlerFactory;
     private final HttpVersionPolicy versionPolicy;
     private final ByteBuffer bytebuf;
+    private final AtomicReference<HttpConnectionEventHandler> protocolHandlerRef;
 
     private volatile boolean expectValidH2Preface;
 
@@ -84,6 +85,7 @@ public class ServerHttpProtocolNegotiator implements HttpConnectionEventHandler
         this.http2StreamHandlerFactory = Args.notNull(http2StreamHandlerFactory, "HTTP/2 stream handler factory");
         this.versionPolicy = versionPolicy != null ? versionPolicy : HttpVersionPolicy.NEGOTIATE;
         this.bytebuf = ByteBuffer.allocate(1024);
+        this.protocolHandlerRef = new AtomicReference<>(null);
     }
 
     @Override
@@ -107,7 +109,9 @@ public class ServerHttpProtocolNegotiator implements HttpConnectionEventHandler
                     final ServerHttp1StreamDuplexer http1StreamHandler = http1StreamHandlerFactory.create(
                             tlsDetails != null ? URIScheme.HTTPS.id : URIScheme.HTTP.id,
                             ioSession);
-                    ioSession.upgrade(new ServerHttp1IOEventHandler(http1StreamHandler));
+                    final HttpConnectionEventHandler protocolHandler = new ServerHttp1IOEventHandler(http1StreamHandler);
+                    ioSession.upgrade(protocolHandler);
+                    protocolHandlerRef.set(protocolHandler);
                     http1StreamHandler.onConnect(null);
                     break;
             }
@@ -140,7 +144,9 @@ public class ServerHttpProtocolNegotiator implements HttpConnectionEventHandler
                 }
                 if (validH2Preface) {
                     final ServerHttp2StreamMultiplexer http2StreamHandler = http2StreamHandlerFactory.create(ioSession);
-                    ioSession.upgrade(new ServerHttp2IOEventHandler(http2StreamHandler));
+                    final HttpConnectionEventHandler protocolHandler = new ServerHttp2IOEventHandler(http2StreamHandler);
+                    ioSession.upgrade(protocolHandler);
+                    protocolHandlerRef.set(protocolHandler);
                     http2StreamHandler.onConnect(bytebuf.hasRemaining() ? bytebuf : null);
                     http2StreamHandler.onInput();
                 } else {
@@ -148,7 +154,9 @@ public class ServerHttpProtocolNegotiator implements HttpConnectionEventHandler
                     final ServerHttp1StreamDuplexer http1StreamHandler = http1StreamHandlerFactory.create(
                             tlsDetails != null ? URIScheme.HTTPS.id : URIScheme.HTTP.id,
                             ioSession);
-                    ioSession.upgrade(new ServerHttp1IOEventHandler(http1StreamHandler));
+                    final HttpConnectionEventHandler protocolHandler = new ServerHttp1IOEventHandler(http1StreamHandler);
+                    ioSession.upgrade(protocolHandler);
+                    protocolHandlerRef.set(protocolHandler);
                     bytebuf.rewind();
                     http1StreamHandler.onConnect(bytebuf);
                     http1StreamHandler.onInput();
@@ -174,15 +182,19 @@ public class ServerHttpProtocolNegotiator implements HttpConnectionEventHandler
 
     @Override
     public void exception(final IOSession session, final Exception cause) {
-        final IOEventHandler sessionHandler = ioSession.getHandler();
-        if (sessionHandler != null) {
-            sessionHandler.exception(session, cause);
+        final HttpConnectionEventHandler protocolHandler = protocolHandlerRef.get();
+        if (protocolHandler != null) {
+            protocolHandler.exception(session, cause);
         }
         session.close(CloseMode.IMMEDIATE);
     }
 
     @Override
     public void disconnected(final IOSession session) {
+        final HttpConnectionEventHandler protocolHandler = protocolHandlerRef.getAndSet(null);
+        if (protocolHandler != null) {
+            protocolHandler.disconnected(ioSession);
+        }
     }
 
     @Override
@@ -193,7 +205,8 @@ public class ServerHttpProtocolNegotiator implements HttpConnectionEventHandler
 
     @Override
     public EndpointDetails getEndpointDetails() {
-        return null;
+        final HttpConnectionEventHandler protocolHandler = protocolHandlerRef.get();
+        return protocolHandler != null ? protocolHandler.getEndpointDetails() : null;
     }
 
     @Override
@@ -208,7 +221,8 @@ public class ServerHttpProtocolNegotiator implements HttpConnectionEventHandler
 
     @Override
     public ProtocolVersion getProtocolVersion() {
-        return null;
+        final HttpConnectionEventHandler protocolHandler = protocolHandlerRef.get();
+        return protocolHandler != null ? protocolHandler.getProtocolVersion() : null;
     }
 
     @Override