BlockHound icon indicating copy to clipboard operation
BlockHound copied to clipboard

Future: detect blocking calls in methods that return Futures

Open vlsi opened this issue 5 years ago • 6 comments

Motivation

https://twitter.com/leventov/status/1209782345756270592 https://cwiki.apache.org/confluence/display/KAFKA/KIP-286%3A+producer.send%28%29+should+not+block+on+metadata+update

Desired solution

It should print an error or raise an exception if a blocking call is executed from a method that returns Future.

Considered alternatives

Inspect code manually

vlsi avatar Dec 25 '19 12:12 vlsi

This may also apply when a method returns other future-like objects: org.reactivestreams.Publisher, j.u.c.Flow.Publisher, RxJava's Observable.

leventov avatar Dec 25 '19 16:12 leventov

‪BlockHound already detects this Kafka's issue:‬

‪https://speakerdeck.com/bsideup/geekout-2019-dont-be-homer-simpson-with-your-reactor?slide=116‬

‪There is no need to support "methods that return Future" because BlockHound operates on Threads level‬ :)

bsideup avatar Dec 25 '19 16:12 bsideup

Oh.

However, someone (in their enterprise app) might still use Futures (e.g. for their do it yourself concurrency). In that case, BlockHound might spot that the app is using an unexpected call sequence.

vlsi avatar Dec 25 '19 17:12 vlsi

Pardon a little Scala code...

def myMethod(): Future[Result] = {
  try {
    val res: Result = rpc()
    successful(res)
  } catch {
    case e: Exception => failed(e)
  }
}

Code pretty much like this was an inspiration for the checklist item.

leventov avatar Dec 25 '19 17:12 leventov

@vlsi

BlockHound works with any JVM code, Futures included.

@leventov if myMethod is called from a non-blocking thread then it will be reported by the current version of BlockHound.

bsideup avatar Dec 25 '19 19:12 bsideup

@bsideup even if we are already in a thread pool designed for blocking operations, this may introduce unnecessary delay: imagine we want to parallelize execution of myMethod() and myMethod2():

myMethod().thenAcceptBoth(myMethod2(), (res, res2) -> { ... });

This code will unnecessarily wait for the blocking operation in the beginning of myMethod() before it even calls to myMethod2() which fires another async operation. This delay could have been avoided if myMethod() returned more promptly.

leventov avatar Dec 26 '19 10:12 leventov