4

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 :).

Eduardo
  • 8,362
  • 6
  • 38
  • 72

4 Answers4

5

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.

Mike Morearty
  • 9,953
  • 5
  • 31
  • 35
Régis Jean-Gilles
  • 32,541
  • 5
  • 83
  • 97
  • I guess it could also have to do with trying to do things a bit differently than what the framework proposes. They have their own json and futures components, but they are a bit underdocumented. I guess that is a general problem with web frameworks for Scala. However, despite being a bit shallow, I would say Scalatra's docs are among the most complete and beginner-friendly in this class. Probably lift has better docs, but it is too heavy for my needs. I am eagerly awaiting the Scalatra MEAP book, but I don't see too much progress in the writing of chapters so far. – Eduardo Jul 03 '13 at 15:45
  • Well, they do have an `AsyncResult` class, but this does not solve the problem of using a thread local. Sadly, you still have to capture the request before at the start of the handler. My gut feeling tells me that async support was added to scalatra as an afterthought – Régis Jean-Gilles Jul 03 '13 at 16:13
4

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.

cmbaxter
  • 35,283
  • 4
  • 86
  • 95
  • Alost surely correct and that is in fact the reason that Scalatra 2.2 has the very convenient `AsyncResult` class. – Randall Schulz Jul 03 '13 at 16:20
  • @RandallSchulz : I do not use `AsyncResult` because I want to send the response immediately. The real response is sent async to a server-socket in the client (it is not a browser but an Adobe Air application). What happens inside the future is fire-and-forget. I use a future and not an actor for the fire-and-forget because actors serialize requests and I would like them to run in parallel. I know I could spawn a short-lived actor but a future seemed less verbose. – Eduardo Jul 03 '13 at 17:47
2

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.

Bruno Grieder
  • 28,128
  • 8
  • 69
  • 101
0

Try to put with FutureSupport on your class declaration like this

class MyServices extends ScalatraServlet with FutureSupport {}

Wins
  • 3,420
  • 4
  • 36
  • 70