Skip to content

Continuations sometimes not called when an asynchronous request is cancelled #77

@hokomo

Description

@hokomo

I believe the way hato wraps the HttpClient future leads to a subtle race condition which sometimes causes the continuations provided to an asynchronous request to never be called when the request is cancelled. The code below should reproduce the issue 99% of the time (in which case you'll see no output):

(let [cf (hato/request
          {:url "https://httpbin.org/status/200" :method :get :async? true}
          #(prn "success! " %)
          #(prn "failure! " %))]
  #_(Thread/sleep 500)
  (.cancel cf true)
  cf)

You can uncomment the sleep and play with the sleep duration to bias the race towards a different outcome: either the request completing successfully (which is not too interesting for us), or the failure continuation actually being called due to cancellation reaching the "root" HttpClient future and triggering the completion of the returned future via the exceptionally handler before it is completed by our own call to cancel.

To be more precise:

  • Let's call the future returned by HttpClient's sendAsync the "root future". It is important to note that since Java 16 that future is guaranteed to be a so-called cancelable future, a concept introduced in sendAsync's docs. Any future derived from a cancelable future is itself a cancelable future. Cancelling a cancelable future will propagate the cancellation upstream until it reaches the root future (unlike standard futures which only propagate the cancellation downstream).
  • Let's call the future returned by hato's request* the "returned future". That future is guaranteed to be cancelable, but it is distinct from the root future because chaining methods like thenApply, exceptionally, etc. always return new futures.
  • Completing a future returned from a chaining method prevents the associated handler from being run (because a future can only be completed once). In other words, when we exceptionally complete the "returned future" by calling cancel, we prevent the handler passed to exceptionally from running when the root future is eventually completed. This is why the failure continuation is never called.
  • It is possible for the propagated cancellation [1, 2] to be faster and complete (exceptionally) both the root future and, via the exceptionally handler, the returned future, before our own cancel. In that case the failure continuation will be called, hence the race condition as mentioned above.

babashka's http-client and java-http-client most likely suffer from the same issue as they're also HttpClient-based and wrap the futures in the same way.

The fix should be straightforward: after calling the chaining methods just return the root future instead. That way we don't risk preemptively completing the future returned by exceptionally and leave that to happen only via the corresponding handler, guaranteeing that one of our continuations will be called eventually. For example:

(doto (.sendAsync http-client http-request bh)
  (.thenApply ...)
  (.exceptionally ...))

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions