5

I was trying to use promises to force serialization of a series of Ajax calls. These Ajax calls are made one for each time a user presses a button. I can successfully serialize the operations like this:

// sample async function
// real-world this is an Ajax call
function delay(val) {
    log("start: ", val);
    return new Promise(function(resolve)  {
        setTimeout(function() {
            log("end: ", val); 
            resolve();
        }, 500);
    });
}

// initialize p to a resolved promise
var p = Promise.resolve();
var v = 1;

// each click adds a new task to 
// the serially executed queue
$("#run").click(function() {
    // How to detect here that there are no other unresolved .then()
    // handlers on the current value of p?
    p = p.then(function() {
        return delay(v++);
    });
});

Working demo: http://jsfiddle.net/jfriend00/4hfyahs3/

But, this builds a potentially never ending promise chain since the variable p that stores the last promise is never cleared. Every new operation just chains onto the prior promise. So, I was thinking that for good memory management, I should be able to detect when there are no more .then() handlers left to run on the current value of p and I can then reset the value of p, making sure that any objects that the previous chain of promise handlers might have held in closures will be eligible for garbage collection.

So, I was wondering how I would know in a given .then() handler that there are no more .then() handlers to be called in this chain and thus, I can just do p = Promise.resolve() to reset p and release the previous promise chain rather than just continually adding onto it.

jfriend00
  • 683,504
  • 96
  • 985
  • 979

5 Answers5

4

I'm being told that a "good" promise implementation would not cause accumulating memory from an indefinitely growing promise chain. But, there is apparently no standard that requires or describes this (other than good programming practices) and we have lots of newbie Promise implementations out there so I have not yet decided if it's wise to rely on this good behavior.

My years of coding experience suggest that when implementations are new, facts are lacking that all implementations behave a certain way and there's no specification that says they should behave that way, then it might be wise to write your code in as "safe" a way as possible. In fact, it's often less work to just code around an uncertain behavior than it is to go test all relevant implementations to find out how they behave.

In that vein, here's an implementation of my code that seems to be "safe" in this regard. It just saves a local copy of the global last promise variable for each .then() handler and when that .then() handler runs, if the global promise variable still has the same value, then my code has not chained any more items onto it so this must be the currently last .then() handler. It seems to work in this jsFiddle:

// sample async function
// real-world this is an Ajax call
function delay(val) {
    log("start: ", val);
    return new Promise(function(resolve)  {
        setTimeout(function() {
            log("end: ", val); 
            resolve();
        }, 500);
    });
}

// initialize p to a resolved promise
var p = Promise.resolve();
var v = 1;

// each click adds a new task to 
// the serially executed queue
$("#run").click(function() {
    var origP = p = p.then(function() {
        return delay(v++);
    }).then(function() {
        if (p === origP) {
            // no more are chained by my code
            log("no more chained - resetting promise head");
            // set fresh promise head so no chance of GC leaks
            // on prior promises
            p = Promise.resolve();
            v = 1;
        }
        // clear promise reference in case this closure is leaked
        origP = null;
    }, function() {
        origP = null;
    });
});
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • You shouldn't forget to do `origP = null`. Don't forget, our assumption is that `then` handlers (and the stuff that they reference) are leaked! :-) Alternatively, just testing for the value of `v` seems to be easier. – Bergi Aug 04 '15 at 18:19
  • @Bergi - `v` is just here for purposes of logging. That isn't part of the normal code. Are you talking about nulling `origP` to clear the reference so it can be GCed in case the `.then()` reference is leaked? In the spirit of what I'm trying to protect against, I guess I should do that. Code updated. – jfriend00 Aug 04 '15 at 18:30
  • Yes, exactly. And probably it shouldn't only be done in the if case, but always :-) – Bergi Aug 04 '15 at 18:33
  • …oh, and what if the promise rejects? Do we need to clear it there as well? – Bergi Aug 04 '15 at 18:34
2

… so that I can then reset the value of p, making sure that any objects that the previous chain of promise handlers might have held in closures will be eligible for garbage collection.

No. A promise handler that has been executed (when the promise has settled) is no more needed and implicitly eligible for garbage collection. A resolved promise does not hold onto anything but the resolution value.

You don't need to do "good memory management" for promises (asynchronous values), your promise library does take care of that itself. It has to "release the previous promise chain" automatically, if it doesn't then that's a bug. Your pattern works totally fine as is.


How do you know when the promise chain has completely finished?

I would take a pure, recursive approach for this:

function extendedChain(p, stream, action) {
     // chains a new action to p on every stream event
     // until the chain ends before the next event comes
     // resolves with the result of the chain and the advanced stream
     return Promise.race([
         p.then(res => ({res}) ), // wrap in object to distinguish from event
         stream                   // a promise that resolves with a .next promise
     ]).then(({next, res}) =>
         next
           ? extendedChain(p.then(action), next, action) // a stream event happened first
           : {res, next:stream};                         // the chain fulfilled first
     );
}
function rec(stream, action, partDone) {
    return stream.then(({next}) =>
        extendedChain(action(), next, action).then(({res, next}) => {
            partDone(res);
            return rec(next, action, partDone);
        });
    );
}

var v = 1;
rec(getEvents($("#run"), "click"), () => delay(v++), res => {
    console.log("all current done, none waiting");
    console.log("last result", res);
}); // forever

with a helper function for event streams like

function getEvents(emitter, name) {
    var next;
    function get() {
        return new Promise((res) => {
            next = res;
        });
    }
    emitter.on(name, function() {
        next({next: get()});
    });
    return get();
}

(Demo at jsfiddle.net)

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Is there a specification that says this or it just sort of "has" to work that way and how would I know that all current implementations have done this properly? – jfriend00 Aug 04 '15 at 16:40
  • @jfriend00: Unfortunately, no js spec says anything about memory management and garbage collection (except *that* ES is a garbage-collected language). So just a sort of "has to work this way" :-) – Bergi Aug 04 '15 at 16:42
  • My guess is that there are promises implementations that leak, at least in some JS implementations with a not-so-good GC, but I don't think there's a way to know this other than by testing. All serious promise libs would be concerned for that though, and will be happy to fix it when you detect a leak. – Bergi Aug 04 '15 at 16:45
  • That's what makes me uncomfortable with relying on a specific implementation behavior. We're relatively "early" in the widespread implementation of promises across many platforms, there's no specification that says it should work this way and I'm writing code expected to work everywhere. My long years as a coder suggest that I should take matters in my own hands to make sure my code works even with an imperfect implementation until I have better evidence that such caution is not required. – jfriend00 Aug 04 '15 at 16:47
  • FYI, it is of little use to an app writer to say that if an implementation was faulty, it would be a bug and they would probably fix it. App writers have to make apps that work on the platforms that are currently deployed, not only on the latest "fixed" platforms. That would be the advantage of not using a built-in Promise implementation I guess, but rather importing your own known Promise replacement implementation (like Bluebird). Then, you'd have a known quantity you could test and code to. – jfriend00 Aug 04 '15 at 16:50
  • I can understand your reluctance. However, a platform can always be implemented in a way that it leaks memory and you can't do anything against that. Avoiding all possible bugs in your environment is impossible, you need to depend on some parts of your runtime. Trying to avoid them is premature defensiveness imo. Instead of working around hypothetical bugs, I'd start by writing simple and maintainable code. Of course, a single dependency like bluebird requires less worrying about the environment. – Bergi Aug 04 '15 at 17:57
  • Well, I'd say that an app writer has to decide which platform capabilities in the intended target of platforms are believed to be reliable and which are more unknown. Obviously, you don't want to reimplement things in the platform that work just fine as already present. And, you don't want your app to depend on things in the platform that are not reliable. – jfriend00 Aug 04 '15 at 18:12
  • 1
    When you are in a gray area and are unsure what can be counted on in the platform for a specific issue and it is complicated and costly to actually try to test all possible platforms for this issue and quick research doesn't turn up any conclusive info on the issue and it's only a few lines of code to defend against a possible unreliability, that may be a wise choice. That's all I'm saying. – jfriend00 Aug 04 '15 at 18:13
1

It is impossible to detect when no more handlers are added.

It is in fact an undecidable problem. It is not very hard to show a reduction to the halting (or the Atm problem). I can add a formal reduction if you'd like but in handwavey: Given an input program, put a promise at its first line and chain to it at every return or throw - assuming we have a program that solves the problem you describe in this question - apply it to the input problem - we now know if it runs forever or not solving the halting problem. That is, your problem is at least as hard as the halting problem.

You can detect when a promise is "resolved" and update it on new ones.

This is common in "last" or in "flatMap". A good use case is autocomplete search where you only want the latest results. Here is an [implementation by Domenic (https://github.com/domenic/last):

function last(operation) {
    var latestPromise = null; // keep track of the latest

    return function () {
        // call the operation
        var promiseForResult = operation.apply(this, arguments);
        // it is now the latest operation, so set it to that.
        latestPromise = promiseForResult;

        return promiseForResult.then(
            function (value) {
                // if we are _still_ the last value when it resovled
                if (latestPromise === promiseForResult) {
                    return value; // the operation is done, you can set it to Promise.resolve here
                } else {
                    return pending; // wait for more time
                }
            },
            function (reason) {
                if (latestPromise === promiseForResult) { // same as above
                    throw reason;
                } else {
                    return pending;
                }
            }
        );
    };
};

I adapted Domenic's code and documented it for your problem.

You can safely not optimize this

Sane promise implementations do not keep promises which are "up the chain", so setting it to Promise.resolve() will not save memory. If a promise does not do this it is a memory leak and you should file a bug against it.

Bimper
  • 270
  • 1
  • 8
  • I don't understand your first paragraph. But, do you understand that I'm not asking to know when the program as a whole is done chaing more `.then()` handlers. I'm just asking how to know from a given `.then()` handler when it is currently the last one in line and there are none after it (at this moment in time). Inside the promise itself, this is already known because when this `.then()` handler returns, the promise knows whether there is a next one to call. – jfriend00 Aug 04 '15 at 14:54
  • As for your last paragraph, I suspected that to possibly be the case, but without a specification that describes that and the ability to verify that all current Promise implementations follow that specification, I did not think it a safe assumption to rely on so I was looking for a way to guarantee that my code would not cause increasing memory usage over time. Plus, it's not a quick thing to test since you're' trying to see if you get an accumulation of memory usage over time. – jfriend00 Aug 04 '15 at 16:19
  • You seem to have forgotten to include `pending` in your code, which is very important. And what parts of [`last.js`](https://github.com/domenic/last/blob/master/lib/last.js) did you "adapt"? – Bergi Aug 04 '15 at 18:14
0

I tried to check if we can see the promise's state in code, apprantly that is only possible from console, not from code, so I used a flag to moniter the status, not sure if there is a loophole somewhere:

  var p
    , v = 1
    , promiseFulfilled = true;



  function addPromise() {
    if(!p || promiseFulfilled){
      console.log('reseting promise...');
      p = Promise.resolve();
    }
    p = p.then(function() {
        promiseFulfilled = false;
        return delay(v++);
    }).then(function(){
      promiseFulfilled = true;
    });
  }

fiddle demo

mido
  • 24,198
  • 15
  • 92
  • 117
  • This does seem to work, though I'm having a hard time understanding why it doesn't have an issue with multiple `.then()` handlers all setting the same global variable. – jfriend00 Aug 04 '15 at 17:13
  • This has a possible race condition, where `addPromise` might be called in between the `promiseFulfilled = true` and the next `promiseFulfilled = false`. – Bergi Aug 04 '15 at 18:11
-1

You could push the promises onto an array and use Promise.all:

var p = Promise.resolve, 
   promiseArray = [], 
   allFinishedPromise;

function cleanup(promise, resolvedValue) {
    // You have to do this funkiness to check if more promises
    // were pushed since you registered the callback, though.
    var wereMorePromisesPushed = allFinishedPromise !== promise;
    if (!wereMorePromisesPushed) {
        // do cleanup
        promiseArray.splice(0, promiseArray.length);
        p = Promise.resolve(); // reset promise
    }
}

$("#run").click(function() {
    p = p.then(function() {
        return delay(v++);
    });
    promiseArray.push(p)
    allFinishedPromise = Promise.all(promiseArray);
    allFinishedPromise.then(cleanup.bind(null, allFinishedPromise));
});

Alternatively, since you know they are executed sequentially, you could have each completion callback remove that promise from the array and just reset the promise when the array is empty.

var p = Promise.resolve(), 
    promiseArray = [];

function onPromiseComplete() {
    promiseArray.shift();
    if (!promiseArray.length) {
        p = Promise.resolve();
    }
}

$("#run").click(function() {
    p = p.then(function() {
        onPromiseComplete();
        return delay(v++);
    });
    promiseArray.push(p);
});

Edit: If the array is likely to get very long, though, you should go with the first option b/c shifting the array is O(N).

Edit: As you noted, there's no reason to keep the array around. A counter will work just fine.

var p = Promise.resolve(), 
    promiseCounter = 0;

function onPromiseComplete() {
    promiseCounter--;
    if (!promiseCounter) {
        p = Promise.resolve();
    }
}

$("#run").click(function() {
    p = p.then(function() {
        onPromiseComplete();
        return delay(v++);
    });
    promiseCounter++;
});
mxdubois
  • 605
  • 8
  • 14
  • Your second method doesn't actually need to store the promises in an array does it since all you're ever looking at is when the array hits zero length? Couldn't it just use an increment/decrement counter (an increment/decrement counter is something I thought about, but was hoping to get promises to maintain the counting for me since that's generally what they do)? – jfriend00 Aug 04 '15 at 04:51
  • Your first option makes my brain hurt trying to figure out how or if it works. I haven't grokked that one yet. – jfriend00 Aug 04 '15 at 04:52
  • Is there potentially an issue with your second option if there are other `.then()` handlers besides the ones I control chained onto a given promise? Our handler would get called so we'd think the chain was done, but there could still be other `.then()` handlers that we didn't explicitly add ourselves that were still active, right? This is why I was worried about using an increment/decrement counter. – jfriend00 Aug 04 '15 at 04:53
  • That's a good point! You don't have to keep the array around. Go with an increment. As to for external callbacks, you don't have to worry about them. They will never be part of this chain, they'll be part of a new chain created by external code. Remember, each call to `then` creates a new promise. Also, callbacks passed to `then` are guaranteed to be called even if the reference to the promise object is lost. The promise is just a handle for the async task you queued. – mxdubois Aug 04 '15 at 05:05
  • I don't think your latter two snippets are working. It would need to be `p = p.then(function() { return delay(v++); }).then(onPromiseComplete);`, currently you're calling `onPromiseComplete` before `delay`, which means that the chain might not have finished. – Bergi Aug 04 '15 at 18:08