softwaremill / sttp

The Scala HTTP client you always wanted!
https://sttp.softwaremill.com
Apache License 2.0
1.45k stars 306 forks source link

FutureAsyncHttpClientHandler hangs the application #26

Closed alexeygolev closed 7 years ago

alexeygolev commented 7 years ago

I'm probably doing something wrong or not doing something obvious. I'm playing around with sttp and I have a simple object extending the App trait that makes a call using FutureAsyncHttpClientHandler backend. It works and I'm getting a response. The problem is the app seems to hang and I can exit it with either System.exit or even providing a custom executor and then shutting it down. I tried doing this

object MyApp {
  def main(args: Array[String]): Unit = {
    import java.util.concurrent.ForkJoinPool
    val executor = new ForkJoinPool()
    implicit val context = ExecutionContext.fromExecutor(executor)
   /* 
   Todo.run uses FutureAsyncHttpClientHandler backend and issues a simple http request 
   returning a Future. Todo.run has an implicit ExecutionContext obviously so the context 
   that I create is used 
  */
   val result = Await.result(Todo.run, 3 seconds)
    try {
      import java.util.concurrent.TimeUnit
      executor.shutdown()
      executor.awaitTermination(1, TimeUnit.MINUTES)
    } catch {
      case e: Exception => println("Something went wrong!")
    }
  }
}

But it didn't change anything

aeons commented 7 years ago

Try calling handler.close.

adamw commented 7 years ago

As @aeons mentioned, I think you need to close the handler as well. Could you paste your full example?

Also, instead of creating a custom ExecutionContext, you can use the global one by importing ExecutionContext.Implicits.global

nrinaudo commented 7 years ago

I think I'm running into something similar:

import com.softwaremill.sttp._
import com.softwaremill.sttp.okhttp._
import scala.concurrent.Future
import scala.concurrent._
import scala.concurrent.duration._

object Evaluate extends App {
  implicit val handler = OkHttpFutureHandler()

  val response: Future[Response[String]] =
    sttp
      .post(uri"http://localhost")
      .send()

  println(Await.result(response, 500.millis))
}

This prints the expected response, then hangs:

nrinaudo commented 7 years ago

At the suggestion of @aeons , I've added a hander.close call at the end:

adamw commented 7 years ago

Indeed, however this only happens for OkHttpFutureHandler. When using OkHttpSyncHandler, the app finishes immediately

adamw commented 7 years ago

I think with async http client taking 4 seconds to close - not something we can influence, sttp is just calling the client's close() method. I suppose terminating and joining all the threads for some reason takes up so much time.

omainegra commented 7 years ago

This is caused by the internal dispatcher of OkHttp, which is used only for async operations, like OkHttpFutureHandler and OkHttpMonixHandler. I just created a PR #32 that fixes this

adamw commented 7 years ago

OkHttp should be fixed in 0.11. AsyncHttp should use close() :)

dsilvasc commented 6 years ago

Another way to keep AsyncHttp from blocking jvm shutdown is to pass in your own thread factory to the netty timer and make the timer thread a daemon thread.

See also https://github.com/apache/incubator-druid/pull/5076

Example config:

  private def defaultClient(options: SttpBackendOptions): AsyncHttpClient =
    new DefaultAsyncHttpClient(
      new DefaultAsyncHttpClientConfig.Builder()
        .setConnectTimeout(options.connectionTimeout.toMillis.toInt)
        // use a daemon thread for the netty timer
        // see also https://github.com/apache/incubator-druid/pull/5076
        .setNettyTimer(new HashedWheelTimer(new ThreadFactoryBuilder().setDaemon(true).build()))
        .build())