[HTTPCORE-601] SSLIOSession.close() is never called for async server handling HTTP1

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 5.0-beta8
• Fix Version/s: 5.0-beta9
• Component/s: HttpCore NIO
• Environment: macOS Mojave, Debian Jessie
• Assignee: Oleg Kalnichevski
• Reporter: Roy Hashimoto
• Created: 13/Sep/19 16:15
• Updated: 13/Oct/19 11:18
• Resolved: 18/Sep/19 08:52

Description

On 5.0, an asynchronous server handling an HTTP1 request never calls SSLIOSession.close(). The attached sample server program exhibits the problem. To test:

1. Set a breakpoint in SSLIOSession.close().
2. Make some requests.

The expectation is for the breakpoint to be triggered when the connection ends. The observed behavior is that the breakpoint is never reached.

The sample program uses Conscrypt (org.conscrypt:conscrypt-openjdk-uber:2.2.1) but the root problem occurs with any JSSE provider including the default. There is an additional bad side effect with Conscrypt, however, which is that the program spins in the selection loop and consumes excessive CPU.

When AbstractHttp1StreamDuplexer.requestShutdown() sets OP_WRITE, this code in onOutput() should call SSLIOSession.close():

https://github.com/apache/httpcomponents-core/blob/44cab548cb4e15d56235aa12eaf0898d028351d0/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/AbstractHttp1StreamDuplexer.java#L366-L373

This doesn't happen because SSLIOSession.isAppOutputReady() returns false here:

https://github.com/apache/httpcomponents-core/blob/44cab548cb4e15d56235aa12eaf0898d028351d0/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java#L139

which happens because status is CLOSING and sslEngine.getHandshakeStatus() returns NEED_WRAP. So onOutput() is not called after requestShutdown() and OP_WRITE remains set. On Conscrypt, selection on OP_WRITE keeps succeeding which causes the spin.

I tried hacking SSLIOSession.isAppOutputReady():
— a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java
@@ -523,8 +523,9 @@ public boolean isAppOutputReady() throws IOException {
         this.session.getLock().lock();
         try

{              return (this.appEventMask & SelectionKey.OP_WRITE) > 0 -                    && this.status == ACTIVE -                    && this.sslEngine.getHandshakeStatus() == HandshakeStatus.NOT_HANDSHAKING; +                    && ((this.status == ACTIVE +                         && this.sslEngine.getHandshakeStatus() == HandshakeStatus.NOT_HANDSHAKING) +                        || this.status == CLOSING);          }

finally

{              this.session.getLock().unlock();          }

This is probably not the appropriate fix, but it does call SSLIOSession.close() and does not spin with Conscrypt so something that achieves that should work.

Issue Links

• links to: GitHub Pull Request #149