How do I write an app using a recursive function that can be started / stopped

Question

I have an app that needs to run a function continuously. That function returns a promise. I want the app to wait until the promise is resolved before it starts the function again.

Additionally, my app needs a start and stop function that causes it to either start running the function, or stop it respectively.

I have a simplified example here:

class App {
  constructor() {
    this.running = false
    this.cycles = 0
  }

  start() {
    this.running = true
    this._run()
  }

  stop() {
    this.running = false
  }

  _run() {
    Promise.resolve().then(() => {
      this.cycles++
    }).then(() => {
      if (this.running) {
        this._run()
      }
    })
  }
}

module.exports = App

My problem is that when I use this, setTimeout seems to give up on me. For example, if I run this:

const App = require("./app")

const a = new App()

a.start()

console.log("a.start is not blocking...")

setTimeout(() => {
  console.log("This will never get logged")
  a.stop()
  console.log(a.cycles)
}, 500)

The output will be:

a.start is not blocking...

And then the code in the setTimeout callback never gets called.

I can try starting running node on my command line and typing directly into the REPL, but after I call a.start() the terminal freezes up and I can no longer type anything.

This kind of thing seems like it should be a pretty common pattern. For example, Express lets you start/stop a server without these issues. What do I need to do to get that behavior?

Answer 1

Your _run() method is infinite. It never stops calling itself unless other code can run and change the value of this.running but just using a .then() is not enough to reliably allow your other setTimeout() code to run because .then() runs at a higher priority than timer events in the event queue.

While .then() is guaranteed to be asynchronous, it will be run at a higher priority than setTimeout() meaning your recursive call just runs indefinitely and your other setTimeout() never gets to run and thus this.running never gets changed.

If instead, you call _run() recursively with a short setTimeout() itself, then your other setTimeout() will get a chance to run. And, since there's really no need to use promises there at all, you can just remove them:

Change it to this:

class App {
  constructor() {
    this.running = false
    this.cycles = 0
  }

  start() {
    this.running = true
    this._run()
  }

  stop() {
    this.running = false
  }

  _run() {
      this.cycles++
      if (this.running) {
         // call recursively after a short timeout to allow other code
         // a chance to run
         setTimeout(this._run.bind(this), 0);
      }
  }
}

module.exports = App

See this other answer for some discussion of the relative priorities between .then(), setImmediate() and nextTick():

Promise.resolve().then vs setImmediate vs nextTick

And more info on the subject in this:

https://github.com/nodejs/node-v0.x-archive/pull/8325

---

The generalized priority hierarchy seems to be:

.then()
nextTick()
other events already in the queue
setImmediate()
setTimeout()

So, you can see from this that .then() jumps in front of other events already in the queue, thus your setTimeout() nevers gets to run as long as their is a .then() waiting to go.

So, if you want to allow other timer events already in the queue to run before your next call to this._run(), you have to use either setImmediate() or setTimeout(). Probably either will work in this case, but since the other events are setTimeout(), I figured using setTimeout() here would guarantee safety since you know a new setTimeout() callback can't jump in front of one that is already a pending event.

Answer 2

Blocking Recursive Promises

This is actually a good example for a "blocking" Promise in connection with recursion.

Your first console.log invocation is executed as expected. It is a synchronous operation and due to the run-to-completion scheduling in Javascript, it is guaranteed to run in the current iteration of the event loop.

Your second console.log is asynchronous and due to the implementation of setTimeout it is attached to the queue of the next iteration of the event loop. However, this next iteration is never reached, because Promise.resolve().then(...) attaches the then callback to the end of the queue of the current iteration. Since this is done recursively, the current iteration is never completed.

Hence your second log, which is queued at the next turn of the event loop, never logs.

With node.js you can reproduce this behavior by implementing a recursive function with setImmediate:

// watch out: this functions is blocking!

function rec(x) {
  return x === Infinity
   ? x
   : (console.log(x), setImmediate(rec, x + 1));
}

Bergi's implementation simply bypasses the normal asynchronous Promise behavior by applying setTimeout to the resolve callback.

Answer 3

Your _run method is running too fast. It's using promises and is asynchronous, so you don't get a stack overflow or something, but the promise task queue is running on a higher priority than the timeout task queue. That's why your setTimeout callback will never be fired.

If you use an actual long-running task instead of Promise.resolve(), it will work.

class App {
  constructor() {
    this.running = false
    this.cycles = 0
  }

  start() {
    if (this.running) {
      return Promise.reject(new Error("Already running"))
    } else {
      this.running = true
      return this._run()
    }
  }

  stop() {
    this.running = false
  }

  _run() {
    return new Promise(resolve => {
      this.cycles++
      setTimeout(resolve, 5) // or something
    }).then(() => {
      if (this.running)
        return this._run()
      else
        return this.cycles
    })
  }
}