Jonathan Jonathan - 2 months ago 6
Node.js Question

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

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

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.