Question

My server application uses Scalatra, with json4s, and Akka.

Most of the requests it receives are POSTs, and they return immediately to the client with a fixed response. The actual responses are sent asynchronously to a server socket at the client. To do this, I need to getRemoteAddr from the http request. I am trying with the following code:

case class MyJsonParams(foo:String, bar:Int)

class MyServices extends ScalatraServlet {
  implicit val formats = DefaultFormats

  post("/test") {
    withJsonFuture[MyJsonParams]{ params =>
      // code that calls request.getRemoteAddr goes here
      // sometimes request is null and I get an exception
      println(request)
    }
  }

  def withJsonFuture[A](closure: A => Unit)(implicit mf: Manifest[A]) = {
    contentType = "text/json"
    val params:A = parse(request.body).extract[A]
    future{
      closure(params)
    }      
    Ok("""{"result":"OK"}""")
  }    
}

The intention of the withJsonFuture function is to move some boilerplate out of my route processing.

This sometimes works (prints a non-null value for request) and sometimes request is null, which I find quite puzzling. I suspect that I must be "closing over" the request in my future. However, the error also happens with controlled test scenarios when there are no other requests going on. I would imagine request to be immutable (maybe I'm wrong?)

In an attempt to solve the issue, I have changed my code to the following:

case class MyJsonParams(foo:String, bar:Int)

class MyServices extends ScalatraServlet {
  implicit val formats = DefaultFormats

  post("/test") {
    withJsonFuture[MyJsonParams]{ (addr, params) =>
      println(addr)
    }
  }

  def withJsonFuture[A](closure: (String, A) => Unit)(implicit mf: Manifest[A]) = {
    contentType = "text/json"
    val addr = request.getRemoteAddr()
    val params:A = parse(request.body).extract[A]
    future{
      closure(addr, params)
    }      
    Ok("""{"result":"OK"}""")
  }    
}

This seems to work. However, I really don't know if it is still includes any bad concurrency-related programming practice that could cause an error in the future ("future" meant in its most common sense = what lies ahead :).

Was it helpful?

Solution 2

I don't know Scalatra, but it's fishy that you are accessing a value called request that you do not define yourself. My guess is that it is coming as part of extending ScalatraServlet. If that's the case, then it's probably mutable state that it being set (by Scalatra) at the start of the request and then nullified at the end. If that's happening, then your workaround is okay as would be assigning request to another val like val myRequest = request before the future block and then accessing it as myRequest inside of the future and closure.

OTHER TIPS

Scalatra is not so well suited for asynchronous code. I recently stumbled on the very same problem as you. The problem is that scalatra tries to make the code as declarative as possible by exposing a dsl that removes as much fuss as possible, and in particular does not require you to explicitly pass data around.

I'll try to explain.

In your example, the code inside post("/test") is an anonymous function. Notice that it does not take any parameter, not even the current request object. Instead, scalatra will store the current request object inside a thread local value just before it calls your own handler, and you can then get it back through ScalatraServlet.request.

This is the classical Dynamic Scope pattern. It has the advantage that you can write many utility methods that access the current request and call them from your handlers, without explicitly passing the request.

Now, the problem comes when you use asynchronous code, as you do. In your case, the code inside withJsonFuture executes on another thread than the original thread that the handler was initially called (it will execute on a thread from the ExecutionContext's thread pool). Thus when accessing the thread local, you are accessing a totally distinct instance of the thread local variable. Simply put, the classical Dynamic Scope pattern is no fit in an asynchronous context.

The solution here is to capture the request at the very start of your handler, and then exclusively reference that:

post("/test") {
  val currentRequest = request
  withJsonFuture[MyJsonParams]{ params =>
    // code that calls request.getRemoteAddr goes here
    // sometimes request is null and I get an exception
    println(currentRequest)
  }
}

Quite frankly, this is too easy to get wrong IMHO, so I would personally avoid using Scalatra altogether if you are in an synchronous context.

I do not know scalatra but at first glance, the withJsonFuture function returns an OK but also creates a thread via the future { closure(addr, params) } call.

If that latter thread is run after the OK is processed, the response has been sent and the request is closed/GCed.

Why create a Future to run you closure ?

if withJsonFuture needs to return a Future (again, sorry, I do not know scalatra), you should wrap the whole body of that function in a Future.

Try to put with FutureSupport on your class declaration like this

class MyServices extends ScalatraServlet with FutureSupport {}

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top