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

Race condition in IOSessionImpl when setting event

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 5.0.3
    • Fix Version/s: 5.1-beta3, 5.1
    • Component/s: HttpCore NIO
    • Labels:
      None

      Description

      There is a race condition in class org.apache.hc.core5.reactor.IOSessionImpl when setting an event through the setEvent(final int op) method: there is a check on the session to ensure it is not closed but this check is performed outside the lock that is later acquired to protect the call on the SelectionKey. If the session is closed just after the check but before getting the lock, then it will throw a CancelledKeyException when trying to perform the call on the SelectionKey.

      I have implemented an HTTP server that use the reactive library (httpcore5-reactive component) and execute the requests in a separate thread pool. I was hit by this issue, here is a stack trace (with traces non specific to HttpCore library removed):

         CancelledKeyException at SelectionKeyImpl.java:71 
         SelectionKeyImpl.java:90 
         IOSessionImpl.java:159 
         InternalDataChannel.java:324 
         AbstractHttp1StreamDuplexer.java:  452 
         ServerHttp1StreamDuplexer.java:136 
         ServerHttp1StreamHandler.java:94 
         ReactiveDataProducer.java:108 
         ReactiveDataProducer.java:100 
         ... traces from publisher implementation using Rx java library... 
         ReactiveServerExchangeHandler.java:91          
         ReactiveServerExchangeHandler.java:83 
         ...

       

      I was able to reproduce it almost systematically with unit test from my application (concurrency issues are very sensitive to time). If necessary I can provide a sample HttpAsyncServer which allows me to reproduce it by introducing a simple Thread.sleep(50) sequence in IOSessionImpl code.

      There is a simple fix: move the check on close status inside the lock-unlock block

         lock.lock();
         try {
             if (isStatusClosed()) {
                  return;
             }
             this.key.interestOps(this.key.interestOps() | op);
         } finally {
           lock.unlock();
         }
      

      Note that 2 other methods of this class are subject to this problem: setEventMask(final int newValue) and clearEvent(final int op). The fix is the same than for the first method.

      I can submit a pull request with the fix(es) if you indicate me in which branch I should submit it.

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              ncapponi Nicolas Capponi
            • Votes:
              0 Vote for this issue
              Watchers:
              5 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 - 1h 20m
                1h 20m