[ 
https://issues.apache.org/jira/browse/HTTPCORE-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16930128#comment-16930128
 ] 

Ryan Schmitt commented on HTTPCORE-601:
---------------------------------------

[~olegk] I think an important exception to this rule is certain HTTP errors 
which leave the connection unusable, such as 408 Request Timeout and 413 
Payload Too Large. The server has to initiate a connection close after 
returning one of these errors, simply because the connection is no longer in 
any valid protocol state.

> SSLIOSession.close() is never called for async server handling HTTP1
> --------------------------------------------------------------------
>
>                 Key: HTTPCORE-601
>                 URL: https://issues.apache.org/jira/browse/HTTPCORE-601
>             Project: HttpComponents HttpCore
>          Issue Type: Bug
>          Components: HttpCore NIO
>    Affects Versions: 5.0-beta8
>         Environment: macOS Mojave, Debian Jessie
>            Reporter: Roy Hashimoto
>            Assignee: Oleg Kalnichevski
>            Priority: Major
>         Attachments: ConscryptTest.java
>
>
> On 5.0, an asynchronous server handling an HTTP1 request never calls 
> {{SSLIOSession.close()}}. The attached sample server program exhibits the 
> problem. To test:
>  # Set a breakpoint in {{SSLIOSession.close()}}.
>  # 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.



--
This message was sent by Atlassian Jira
(v8.3.2#803003)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to