CODEWITHSUNDEEP
scalaakkafuture
lolski
lolski

Reputation: 17501

Actor and Future: Referring to an actor message within onComplete

While refactoring actor codes written by some other programmers, I encountered the usage of Future.onComplete callback within actor A, which goes against the best practice of using akka.pattern.pipe. It's a bad idea since it exposes the possibility of race conditions as the Future instance might be executed on a different thread.

Looking at the code, we see that there are neither sender nor any mutable vars being referred within the onComplete block so it seems pretty safe, at least for this specific occasion. However, one grey area that leaves me wondering are the references to url and especially text.

Is it possible that similar to the Closing Over An Akka Actor Sender In The Receive problem, a race condition happens such that at the time when the onComplete callback is invoked, the value of text already refers to a different actor message, causing all hells to break loose?

class B extends akka.actor.Actor {
  def receive = {
    case urlAndText: (String, String) => // do something
  }
}

class A extends akka.actor.Actor {
  case class Insert(url: String)

  def fileUpload(content: String): String = ??? // returns the url of the uploaded content
  val b = context.actorOf(Props(classOf[B]))
  def receive = {
    case text: String =>
      Future {
        fileUpload(text)
      } onComplete {
        case Success(url) =>
          b ! Insert(url, text) // will this be
      }
  }
}

Upvotes: 5

Views: 288

Answers (2)

Shadowlands
Shadowlands

Reputation: 15074

The reference to text should be fine. The difference is that each "instance" of text is a new variable bound to the scope of the current match block (starting at case text ...). Thus the Future created closes over the value of text just fine.

This is different from sender (or sender() when de-sugared) which is actually a method defined on the Actor trait which returns the ActorRef of the sender of the most recent message received by the actor on which it is called, and so can give a different value when called later (when the Future's onComplete is finally called).

You are right to be suspect about the use of onComplete, too. A better option would be:

case text: String =>
  Future {
    fileUpload(text)
  } map { url => 
    Insert(url, text) 
  } pipeTo b

Now failures will also be sent on to b, rather than being quietly swallowed.

Upvotes: 5

Gavin Schulz
Gavin Schulz

Reputation: 4716

In this case you aren't going to run into any race conditions although as you noted it is not a particularly good idea to structure code this way in general.

The references to both url and text are fine. The value for url is simply an extraction of a successfully completed Future and that doesn't change whether you're in an Actor or not. The value for text is an immutable string and closing over that value in a Future shouldn't cause problems because that string instance is immutable.

As you noted closing over sender or a var is a problem and that's because being mutable objects those values could change by the time the Future completes unlike immutable values which will remain constant even when you close over them.

Upvotes: 1

Related Questions