4

I have a class method that chains together other methods in the class, and also calls a method on another instance of the class:

class Thing {
    doSomething(nextThing) {
        return new Promise((resolve) =>
            this.initialize()
                .then(() => this.doA())
                .then(() => {
                    nextThing.initialize(); // call initialize() on another instance
                    return this.doB();
                })
                .then(() => this.doC())
                .then(resolve)
        );
    }

    initialize() {
        return new Promise((resolve) => {
            // take a long time to do something
            // ...
            // ...
            resolve();
        });
    }

    doA() { return new Promise((resolve) => resolve()); }

    doB() { return new Promise((resolve) => resolve()); }

    doC() { return new Promise((resolve) => resolve()); }
}

const thing1 = new Thing();
const thing2 = new Thing();

thing1.doSomething(thing2);

Calling the function on the other class instance locks up the flow of the chain, however; this.doB() and nextThing.initialize() will run simultaneously (as desired), but this.doC() won't run until nextThing.initialize() has resolved.

What's the right way to make sure this flows as expected, with nextThing.initialize() and this.doB() starting simultaneously, and starting this.doC() immediately after this.doB() resolves? It's okay if nextThing.initialize() resolves after this.doC().

j0hnm4r5
  • 407
  • 3
  • 17
  • 4
    JavaScript is still single threaded. So if you have process that just takes a long time, stuff can still only happen after that process is done. – Felix Kling Jun 12 '18 at 20:08
  • Are you sure `this.doB` isn't resolving after `initialize`? – J. Pichardo Jun 12 '18 at 20:12
  • 2
    Think of promise constructors like procrastination. It can put off doing the hard work until later, but at some point it still has to do the hard work, and at that point it will still block the thread. – Patrick Roberts Jun 12 '18 at 20:16
  • 2
    @J.Pichardo: It actually looks like `this.doB` isn't even running until `nextThing.initialize` finishes. – j0hnm4r5 Jun 12 '18 at 20:18
  • @PatrickRoberts, I was under the impression I could run both `this.doB`/`this.doC` and `nextThing.initialize` in parallel? Or is it that I can kick them off in parallel, but actual execution is going to run in series? – j0hnm4r5 Jun 12 '18 at 20:22
  • @j0hnm4r5 there is no real parallelism in javascript, what you see is the callback queue being cleared, whenever there is space in the stack. – J. Pichardo Jun 12 '18 at 20:24
  • If you are using Node you could start a child process. – Felix Kling Jun 12 '18 at 20:26
  • @j0hnm4r5 you're not even "kicking them off in parallel". As you observed, the function passed to the promise constructor is invoked synchronously, so you're not even delaying it by a tick, and since you're not making anything dependent on its completion, it's equivalent to invoking the function directly without wrapping it in a promise constructor. – Patrick Roberts Jun 12 '18 at 20:30
  • Man I tried to run this and there are missing `{` and `})` ... – Cody G Jun 12 '18 at 20:36
  • @CodyG. Sorry! I wrote it by hand in the editor, trying to simplify my actual code. It should be fixed now! – j0hnm4r5 Jun 12 '18 at 20:48
  • Are you still confused at anything? – Cody G Jun 12 '18 at 20:56
  • 1
    Avoid [explicit construction](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it) – Benjamin Gruenbaum Jun 12 '18 at 20:57
  • Also, promises are useless for synchronous actions. If your promise constructor takes a long time to execute - consider offloading the work somewhere else (another worker or process) – Benjamin Gruenbaum Jun 12 '18 at 20:58
  • I took your code, added a bunch of `console.log`s to it, added some `setTimeout`s to simulate "work being done", and threw it in a fiddle: https://jsfiddle.net/r54tuo8j/ Seems to do what I would expect it to do. – Heretic Monkey Jun 12 '18 at 22:04
  • FYI, Benjamin is saying that `doSomething` could just be `return this.initialize().then()`. – Cody G Jun 12 '18 at 22:19
  • @MikeMcCaughan Hmm. That does seem to be working exactly as expected. Which means my problem lies elsewhere. Thank you for your help! I should have known to actually run the code I was asking for help on. – j0hnm4r5 Jun 13 '18 at 14:21
  • @MikeMcCaughan Actually, wait. I tried replacing the `setTimeout` in `initialize` with a while loop that increments a variable to a high number, and it blocked just as before. Is it just because of the way `setTimeout` works? – j0hnm4r5 Jun 13 '18 at 14:30
  • A while loop is not asynchronous; setTimeout is. – Heretic Monkey Jun 13 '18 at 14:32
  • See also [Correct way to write a non-blocking function in Node.js](https://stackoverflow.com/questions/53876344/proper-way-to-write-nonbloking-function-in-node-js) – Bergi Jan 31 '21 at 22:36

1 Answers1

9

When you do this structure:

return new Promise(resolve => {
    // run some long synchronous piece of code
    resolve(...);
});

Here's what happens.

  1. A new promise object is created
  2. The promise executor callback is called synchronously as part of the promise constructor execution
  3. Your long-running synchronous code is called om that executor callback
  4. You call resolve(...) to resolve the previously created promise
  5. You return from the promise executor
  6. The promise constructor returns
  7. You return from the host function and the line of code after this function call will get to run
  8. Sometime later (after the current piece of Javascript returns control back to the system), the .then() handlers are called on the previous promise.

So, a promise calls the executor callback synchronously. It doesn't allow you to "run anything in the background". Javascript is still single threaded.

You can't use a promise to make synchronous code into asynchronous code. You can use some promise techniques to change the scheduling of when code runs, but synchronous code in Javascript is still synchronous and blocking code in Javascript no matter when it runs.

Promises are purely a notification system for notifying you when some other operation has told a promise that it is now resolved or rejected. They don't magically convert synchronous code into asynchronous code.

So, bottom line, you can't use promises to take a synchronous, long-running initialize() function and somehow make it non-blocking or asynchronous.

What's the right way to make sure this flows as expected, with nextThing.initialize() and this.doB() starting simultaneously,

If nextThing.initialize() is synchronous and blocking, it can't run simultaneous with anything. node.js runs your Javascript single threaded. One piece of Javascript running at a time. Promises can't change that.

and starting this.doC() immediately after this.doB() resolves

Since this.doB() and this.doC() both return promises, then you chain the promises with chained .then() handlers to sequence those operations. Your code appears to already do that.

For info about options for off-loading long running synchronous code outside the current node.js single Javascript thread, see this other answer:

Make time intensive function asynchronous


FYI, perhaps this is just pseudo code, but there's never a reason to so this:

return new Promise((resolve) => resolve());

You can instead just do:

return Promise.resolve();.
jfriend00
  • 683,504
  • 96
  • 985
  • 979