Uploaded image for project: 'HttpComponents HttpCore'
  1. HttpComponents HttpCore
  2. HTTPCORE-601

SSLIOSession.close() is never called for async server handling HTTP1

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 5.0-beta8
    • Fix Version/s: 5.0-beta9
    • Component/s: HttpCore NIO
    • Labels:
      None
    • Environment:
      macOS Mojave, Debian Jessie

      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.

        Attachments

        1. ConscryptTest.java
          4 kB
          Roy Hashimoto

          Issue Links

            Activity

              People

              • Assignee:
                olegk Oleg Kalnichevski
                Reporter:
                rhashimoto Roy Hashimoto
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 3h
                  3h