2

I wrote some code below that uses promises and the easiest way I could find to write it was using a Deferred object instead of the usual Promise executor function because I need to resolve the promise from outside the executor. I'm wondering if there's an accepted design pattern based on the Promise executor function for a problem like this that doesn't use a deferred-like solution? Can it be done without having to resolve the promise from outside the promise executor?

Here are the details.

I have a project that uses a set of Worker Threads and various parts of the code that want to use a Worker Thread from time to time. To manage that, I've created a simple WorkerList class that keeps a list of the available Worker Threads. When someone wants to use one, they call get() on it and that returns a promise that resolves to a Worker Thread. If a worker thread is available immediately, the promise resolves immediately. If all worker threads are in use (and thus the list of available workers is empty), then the promise doesn't resolve until one is later put back into the available list via the add(worker) method.

This WorkerList class has only two methods, add(worker) and get(). You get() a worker and when you're done with it, you add(worker) it back. When you add(worker) it back, the class checks to see if there are any tasks waiting for an available Worker. If there, are, it resolves their promise with an available Worker. That resolving of someone else's promise is where the Deferred was used.

Here's the code for the WorkerList:

class WorkerList {
    constructor() {
        this.workers = [];
        this.deferredQueue = [];
    }
    add(worker) {
        this.workers.push(worker);

        // if someone is waiting for a worker,
        // pull the oldest worker out of the list and
        // give it to the oldest deferred that is waiting
        while (this.deferredQueue.length && this.workers.length) {
            let d = this.deferredQueue.shift();
            d.resolve(this.workers.shift());
        }
    }
    // if there's a worker, get one immediately
    // if not, return a promise that resolves with a worker
    //    when next one is available
    get() {
        if (this.workers.length) {
            return Promise.resolve(this.workers.shift());
        } else {
            let d = new Deferred();
            this.deferredQueue.push(d);
            return d.promise;
        }
    }
}

And, here's the Deferred implementation:

function Deferred() {
    if (!(this instanceof Deferred)) {
        return new Deferred();
    }
    const p = this.promise = new Promise((resolve, reject) => {
        this.resolve = resolve;
        this.reject = reject;
    });
    this.then = p.then.bind(p);
    this.catch = p.catch.bind(p);
    if (p.finally) {
        this.finally = p.finally.bind(p);
    }
}
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Sorry, if this seems dumb - I'm waiting for coffee to kick in. Am I reading this correctly - `Deferred` is basically a parameterless `Promise` that you use to resolve/reject it from outside the executor? So, the question is "is there a standard way to resolve a Promise from the outside the executor"? – VLAZ Apr 22 '20 at 06:30
  • @VLAZ - I'm more interested in whether there's a different way to implement this interface that doesn't have to resolve the promise from outside the executor. I've seen people trash the concept of a deferred and I'm wondering if there is a good alternative to using a deferred-like solution. – jfriend00 Apr 22 '20 at 06:32
  • Ah, I see. I'm not really sure there is. Unless brain is failing me right now, this seems like a chicken and egg problem if it has to be inside the executor, since you'd need to know when to resolve, but you cannot know when creating the Promise. Might be possible using a different construct perhaps. Something like a Future/Task which represent an async *operation* rather than a value. But I'm not really sure what the best way is. – VLAZ Apr 22 '20 at 06:38
  • @VLAZ - I imagined deriving from an `EventEmitter` and having `.add()` emit something that the promise executor was listening for, but I wasn't sure how to keep the whole system FIFO since every waiting promise would be listening for the same event and it quickly seemed more complicated than what's here. – jfriend00 Apr 22 '20 at 06:41
  • Hmm, you're right that does sound promising. I didn't even think about having the executor being event driven. Still not sure what a good way to do this is but it's much better than what I had in mind. Also, not sure if there is a standard way to create that. Hopefully, there is. – VLAZ Apr 22 '20 at 06:45
  • when is the while loop necesary? Since workers are consumed upon get, when "add" enters, either there is no deferred, and if there is, it means there is no worker so you can only "resolve" one deferred ? – grodzi Apr 22 '20 at 07:52
  • 1
    @grodzi - The while loop is not necessary if nobody ever messes with the instance variables outside the two methods. But, I did imagine a constructor that might prefill the instance data. So, I decided that the `while` loop was really no more code than a simple `if` so I put it that way. – jfriend00 Apr 22 '20 at 07:53

2 Answers2

4

Maybe the below is just a poor man's approach to deferreds, and doesn't really get to the crux of the matter, but instead of a queue of deferreds, you could just keep a queue of resolver functions.

This saves a small amount of code over your approach and avoids explicitly using Deferreds.

I don't know if there is an established pattern for this, but this in itself seems like a reusable pattern for maintaining an asynchronous pool of objects, so rather than calling it WorkerList, you could name it AsyncPool, and then compose that as a reusable piece within your WorkerList:

class AsyncPool {
    constructor() {
        this.entries = [];
        this.resolverQueue = [];
    }
    add(entry) {
        console.log(`adding ${entry}`);
        this.entries.push(entry);

        // if someone is waiting for an entry,
        // pull the oldest one out of the list and
        // give it to the oldest resolver that is waiting
        while (this.resolverQueue.length && this.entries .length) {
            let r = this.resolverQueue.shift();
            r(this.entries.shift());
        }
    }
    // if there's an entry, get one immediately
    // if not, return a promise that resolves with an entry
    //    when next one is available
    get() {
        return new Promise((r) => 
            this.entries.length
                ? r(this.entries.shift())
                : this.resolverQueue.push(r)
        );
    }
}


let pool = new AsyncPool();

pool.add('Doc');
pool.add('Grumpy');
pool.get().then(console.log);
pool.get().then(console.log);
pool.get().then(console.log);
pool.get().then(console.log);

// add more entries later
setTimeout(() => pool.add('Sneezy'), 1000);
setTimeout(() => pool.add('Sleepy'), 2000);
JLRishe
  • 99,490
  • 19
  • 131
  • 169
  • That's clever. It isn't quite as flexible if you wanted to also be able to reject the promise (imagine an external timeout or a problem with workers), but it does meet the needs of what I described. I would have to say that this `this.waiterQueue.shift()(this.entries.shift());` leaves one scratching their head for awhile trying to figure out what that is doing. I get it now, but it wasn't immediately obvious. – jfriend00 Apr 22 '20 at 07:47
  • @jfriend00 Yeah, I guess I overdid it there. I've changed it back to two separate lines like you had. – JLRishe Apr 22 '20 at 07:49
  • Much better that way. `this.waiterQueue` could be `this.resolverQueue` too. – jfriend00 Apr 22 '20 at 07:50
  • @jfriend00 Makes sense. I'll change that too. But yeah, I agree if you want a way to fail the promises then it seems like Deferreds would be your best bet since they package the resolution and rejection functions together. – JLRishe Apr 22 '20 at 07:56
2

Here is one solution that doesn't expose the promise resolver function anywhere outside the promise executor function.

Following up on my comment to my own question about an event-based solution, here's what I came up with. It uses a triggered event and an event listener to cause an action inside the promise executor function.

class WorkerList extends EventEmitter {
    constructor() {
        this.workers = [];
    }
    add(worker) {
        this.workers.push(worker);
        // notify listeners that there's a new worker in town
        this.emit('workerAdded');
    }
    // if there's a worker, get one immediately
    // if not, return a promise that resolves with a worker
    //    when next one is available
    get() {
        if (this.workers.length) {
            return Promise.resolve(this.workers.shift());
        } else {
            return new Promise(resolve => {
                const onAdded = () => {
                    if (this.workers.length) {
                        this.off('workerAdded', onAdded);
                        resolve(this.workers.shift());
                    }
                }

                this.on('workerAdded', onAdded);
            });
        }
    }
}

I was initially concerned about maintaining FIFO ordering so that the first one to call get() gets the next worker available. But, because eventListeners are called in the order they were added, I think this would actually achieve FIFO order. If there are multiple calls to get(), they will all get notified about the workerAdded, but after the first one handles the message and takes the worker, the others will just find no worker left for them so their listener will stay attached waiting for a future workerAdded message when there is a worker for them (when their listener gets to be first in line).

I don't think I necessarily like this better than the other options shown, but it is an alternative and doesn't use Deferreds or even expose the resolve handler outside the executor function.


As was suggested, this could also be done where the eventEmitter is an instance variable rather than a base class:

class WorkerList {
    constructor() {
        this.workers = [];
        this.emitter = new EventEmitter();
    }
    add(worker) {
        this.workers.push(worker);
        // notify listeners that there's a new worker in town
        this.emitter.emit('workerAdded');
    }
    // if there's a worker, get one immediately
    // if not, return a promise that resolves with a worker
    //    when next one is available
    get() {
        if (this.workers.length) {
            return Promise.resolve(this.workers.shift());
        } else {
            return new Promise(resolve => {
                const onAdded = () => {
                    if (this.workers.length) {
                        this.emitter.off('workerAdded', onAdded);
                        resolve(this.workers.shift());
                    }
                }

                this.emitter.on('workerAdded', onAdded);
            });
        }
    }
}
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • This is very cool, but I question how much sense it makes for the `WorkerList` itself to be an EventEmitter when all of the events are taking place within its inner workings. It seems you could achieve the same thing by having it _contain_ an EventEmitter instead of inheriting one. – JLRishe Apr 22 '20 at 10:10
  • @JLRishe - I agree, that could go either way. If there's no outside use for it now or in the future, then it should probably just be an instance variable (though that does entail more verbose code to make it work that way). – jfriend00 Apr 22 '20 at 10:14
  • 2
    @JLRishe - I added your suggestion as an option. – jfriend00 Apr 22 '20 at 10:23