Surprised by Re-entrant Code

Some of the trickiest bugs I’ve seen happened because a function unexpectedly occurred multiple times on the call stack.

Re-entrant Listeners

To illustrate, consider a TextField widget that implements the listener pattern:

class TextField {
  var listeners = mutableListOf<Listener>()

  var value: String = ""
    set(value) {
      val previous = field
      if (value == previous) return // No change.

      field = value
      for (listener in listeners) {
        listener.textChanged(previous, value)
      }
    }

  fun interface Listener {
    fun textChanged(from: String, to: String)
  }
}

Listeners are very powerful! For example, we could use a listener to force the text field’s contents to uppercase:

  val textField = TextField()

  textField.listeners += TextField.Listener { from, to ->
    textField.value = to.uppercase()
  }

Or we could use a listener to log changes to the text field over time:

  textField.listeners += TextField.Listener { from, to ->
    println("""changed from "$from" to "$to"""")
  }

And because that first listing shows support for a list of listeners, we can use both at the same time. Let’s see what happens when we do that:

  val textField = TextField()

  textField.listeners += TextField.Listener { from, to ->
    textField.value = to.uppercase()
  }

  textField.listeners += TextField.Listener { from, to ->
    println("""changed from "$from" to "$to"""")
  }

  textField.value = "hello"

This program prints the following:

changed from "hello" to "HELLO"
changed from "" to "hello"

These print statements are in the wrong order?! Well this fucking sucks. What the fuck.

Tracing through shows an event sequence like this:

textField.value = "hello"
  notify listeners change from "" to "hello"
    uppercase listener notified change from "" to "hello"
      textField.value = "HELLO"
        notify listeners change from "hello" to "HELLO"
          uppercase listener notified change from "hello" to "HELLO"
            textField.value = "HELLO"
            no change
          logging listener notified change from "hello" to "HELLO"
            print(changed from "hello" to "HELLO")
    logging listener notified change from "" to "hello"
      print(changed from "" to "hello")

The functions all look reasonable locally, but produce a very unreasonable result in aggregate. Yuck.

The most suspicious thing here is the uppercase listener: mutating an observed value in a callback is dangerous because it can lead to strange outcomes like this. But such things aren’t generally enforced!

We’ll also get burned by unsubscribing in a callback:

  val nameField = TextField()
  nameField.value = "Jesse"

  val saveButton = Button("Save")
  saveButton.enabled = false

  val nameChangedListener = object : TextField.Listener {
    override fun textChanged(from: String, to: String) {
      saveButton.enabled = true
      nameField.listeners -= this
    }
  }
  nameField.listeners += nameChangedListener

  nameField.listeners += TextField.Listener { from, to ->
    println("""changed from "$from" to "$to"""")
  }

  nameField.value = "Jesse W"

I’d expect this program to print one name change, but it doesn’t! The size of listeners changes mid-iteration, and the loop terminates before visiting the 2nd element. Yuck.

It’s fragile code like this that makes me very happy to see listeners disappearing from modern UI frameworks. Unidirectional data flow patterns and tools like Kotlin Flows avoid listener traps.

Unfortunately, reentrant functions aren’t just a problem for listeners. The frameworks I love support user-provided callbacks for features like event listeners, interceptors, and adapters. There’s often undocumented things that behave strangely when used within a callback.

Re-entrant Authenticators

For example, OkHttp’s Authenticator callback is used to look up Authorization headers for a forthcoming HTTP request. A simple implementation might make an HTTP request to retrieve a session token from a server. And that request? It’ll also be sent to the Authenticator, yielding surprise recursion!

Once you avoid infinite recursion you might stumble into another gotcha: OkHttp limits how many concurrent calls it’ll make to a single server, and during authentication you might be consuming two of these permits. Get unlucky and your HTTP calls will stall because each of them holds one permit and is waiting for another to become available; a dining philosophers deadlock. Mitigate this by using synchronous calls in your Authenticator; Call.execute() isn’t subject to concurrent call limits.

Re-entrant JSON Adapters

To convert any object to JSON, Moshi first builds a JsonAdapter for that class, which recursively requires adapters for all of its member properties.

This works great for most business objects: an OrderPizzaRequest needs a Pizza adapter, and that needs a List<Topping> adapter. But for generic data structures things get interesting:

data class TreeNode(
  val label: String,
  val left: TreeNode?,
  val right: TreeNode?,
)

The adapter aught to look like this, but we have a chicken-egg problem:

class TreeNodeJsonAdapter(
  val labelAdapter: JsonAdapter<String>,
  val leftAdapter: JsonAdapter<TreeNode?>,
  val rightAdapter: JsonAdapter<TreeNode?>,
): JsonAdapter<TreeNode> {
  ...
}

Moshi’s mitigation is pretty special! It builds a JsonAdapter that forwards all the toJson() and fromJson() calls to a lazily-provided delegate, then it uses this to build a TreeNodeJsonAdapter before it knows how to serialize its properties.

Defending against Re-entrant Code

My preferred strategy is to forbid it! I can detect unexpected recursion or re-entrant behavior and explode.

class TextField {
  var listeners = mutableListOf<Listener>()

  private var changing = false
  
  var value: String = ""
    set(value) {
      require(!changing) { "unexpected re-entrant call" }
      changing = true
      try {
        val previous = field
        if (value == previous) return // No change.

        field = value
        for (listener in listeners) {
          listener.textChanged(previous, value)
        }
      } finally {
        changing = false
      }
    }
  
  ...
}

And when I really need to support re-entrant calls? My only strategy is to be careful and to write lots of tests!