Discuss
  • Cancel Promise pattern (no cancellable promises)

Cancel Promise pattern (no cancellable promises)

# Jan-Ivar Bruaroey (8 years ago)

This is an alternative to cancellable promises that relies entirely on existing JavaScript.

I'm posting this here in hopes to bring the discussion back to practical use cases and minimal needs.

Example:

Here's a setTimeout wrapper with cancellation, using a regular promise as a cancellation token.

let wait = (ms, cancel) => {
   let id;
   return Promise.race([
     new Promise(resolve => id = setTimeout(resolve, ms)),
     (cancel || new Promise(() => {})).then(() => clearTimeout(id))
   ]);
};

function CancelledError() {
   return Object.assign(new Error("The operation was cancelled."), {name: "CancelledError"});
}

// Demo:

async function foo() {
   try {
     let token = new Promise((r, e) => cancel.onclick = () => e(new CancelledError()));
     await wait(500);
     console.log("Wait 4 seconds...");
     await wait(4000, token);
     console.log("Then wait 3 more seconds...");
     await wait(3000, token);
     console.log("Done.");
   } catch (e) {
     if (e.name != "CancelledError") throw e;
     console.log("User cancelled");
   }
}
foo();

Here's the es6 version to run: jsfiddle.net/jib1/jz33qs32

Things to note:

  • Cancellation is targeted to specific operations (no "cancel chain" ambition).
  • Token can be reused down the chain.
  • Cancellation is propagated using a regular (new) CancellationError (no third rail).
  • It is up to the caller whether to treat cancellations as non-exceptional.
  • Basic Promise.race pattern works even to wrap APIs that aren't cancellable (stop waiting)
  • Pattern allows substituting any error (though I hope we standardize CancelledError).
  • Pattern allows chain resumption by resolving token with any desired value instead.

I don't think this group needs to develop much here, maybe standardize CancelledError, and have fetch() take a cancel promise argument like wait() does in the example above.

I'm open to hearing what use-cases are not be covered by this.

# Jan-Ivar Bruaroey (8 years ago)

Likely this would be more convincing without a bug. Here is the correct wait function:

let wait = (ms, cancel = new Promise(() => {})) => {
   let id, both = x => [x, x];
   cancel.then(...both(() => clearTimeout(id)));
   return Promise.race([new Promise(resolve => id = setTimeout(resolve, ms)), cancel]);
};

For caller flexibility, we want to cancel on any activity of the cancel promise, which would have been more apparent in an example that actually relied on clearTimeout working. Fiddle updated. PTAL!

# Bergi (8 years ago)

Jan-Ivar Bruaroey wrote:

I'm posting this here in hopes to bring the discussion back to practical use cases and minimal needs.

Sorry, did we leave that somewhere? :-)

But you've got some good and important points.

Things to note:

  • Cancellation is targeted to specific operations (no "cancel chain" ambition).

I'd however love to be able to cancel specific chaining operations, i.e. then callbacks.

  • Token can be reused down the chain.
  • Cancellation is propagated using a regular (new) CancellationError (no third rail).
  • It is up to the caller whether to treat cancellations as non-exceptional.

Very important. I'd even go so far to let the caller only treat cancellations that he caused himself as non-exceptional.

  • Basic Promise.race pattern works even to wrap APIs that aren't cancellable (stop waiting)
  • Pattern allows substituting any error (though I hope we standardize CancelledError).
  • Pattern allows chain resumption by resolving token with any desired value instead.

I'm not sure what you mean by "resumption". And what would that value be used for?

I'm open to hearing what use-cases are not be covered by this.

Looking forward to your feedback about using a regular promise as a cancellation token.

A crucial problem that promises don't solve is synchronous inspection. If my operation was cancelled, I'd like to know immediately (before starting further work) about it, instead of waiting another tick to be notified.

But the fundamental problem with promises as cancellation tokens is memory leaks. In your example, if the cancel button isn't clicked for 10 seconds, the token promise will reference 3 () => clearTimeout(id)

callbacks which close over their respective ids. Three functions and three integer ids doesn't sound like much, but in real applications with long-running un-cancelled operations a token could accumulate quite an amount of resources which cannot be collected. A clever programmer might make the callbacks become cheap no-ops, but still at least the functions themselves will consume memory. For the simple programmer, we need an automatic (not error-prone) unsubscription mechanism once the respective cancellable operation ended.

Kind , Bergi

# Jan-Ivar Bruaroey (8 years ago)

On 10/27/16 4:25 PM, Bergi wrote:

But you've got some good and important points.

Thanks!

Things to note:

  • Cancellation is targeted to specific operations (no "cancel chain" ambition).

I'd however love to be able to cancel specific chaining operations, i.e. then callbacks.

If you try the fiddle - jsfiddle.net/jib1/jz33qs32 - you'll see cancelling terminates the chain. If you intersperse non-cancellable operations, there'd be a delay if cancel is detected during those.

  • Pattern allows chain resumption by resolving token with any desired value instead.

I'm not sure what you mean by "resumption". And what would that value be used for?

Just basic Promise.race. If users were to resolve the cancel promise instead of rejecting it, it'd cancel the specific operation and inject a replacement success value instead of failing the remaining chain.

I'm not claiming it has utility, just avoiding inventing things. Perhaps:

 fetch("http://flo.ra/dailyflower.png", {cancel: wait(5000).then(()

=> fetch("lily.png")})

A crucial problem that promises don't solve is synchronous inspection. If my operation was cancelled, I'd like to know immediately (before starting further work) about it, instead of waiting another tick to be notified.

I think it'd be odd to observe cancellation and not success nor failure, so this seems orthogonal.

But the fundamental problem with promises as cancellation tokens is memory leaks. In your example, if the cancel button isn't clicked for 10 seconds, the token promise will reference 3 () => clearTimeout(id) callbacks which close over their respective ids. Three functions and three integer ids doesn't sound like much, but in real applications with long-running un-cancelled operations a token could accumulate quite an amount of resources which cannot be collected. A clever programmer might make the callbacks become cheap no-ops, but still at least the functions themselves will consume memory. For the simple programmer, we need an automatic (not error-prone) unsubscription mechanism once the respective cancellable operation ended.

Thanks for the links. I think I'm in the camp of not being concerned about this. Recall I'm not proposing new functionality, just using promises, so this stands to benefit from optimizations browsers ought to make already, without needing special attention. Once browsers optimize:

 function poll() { return isDone() || wait(1000).then(poll); }

I'll worry about this. ;)

.: Jan-Ivar :.

# Bergi (8 years ago)

Jan-Ivar Bruaroey wrote:

On 10/27/16 4:25 PM, Bergi wrote:

I'd however love to be able to cancel specific chaining operations, i.e. then callbacks.

If you try the fiddle - jsfiddle.net/jib1/jz33qs32 - you'll see cancelling terminates the chain. If you intersperse non-cancellable operations, there'd be a delay if cancel is detected during those.

Yes, that's what I mean. Sure, I could use Promise.race to get the cancellation even if the non-cancellable operation resumes, but that's quite ugly:

Promise.race([ promise() …chain…, cancelToken ]).then(callback);

especially when you'll need to nest that pattern. Instead, I'd just like to write

promise() …chain… .then(callback, cancelToken)

with the same behaviour.

A crucial problem that promises don't solve is synchronous inspection. If my operation was cancelled, I'd like to know immediately (before starting further work) about it, instead of waiting another tick to be notified.

I think it'd be odd to observe cancellation and not success nor failure, so this seems orthogonal.

I meant the producer would want to observer the cancellation so that he doesn't attempt to resolve the promise.

But yeah, observing cancellation vs success/failure is another problem that would benefit from inspection. Let's say I have a cancel token and a promise chain. Now I want to do exactly one of three different things, depending on what happens first: the operation is cancelled, the promise is rejected, or the promise fulfills. How do I do that?

But the fundamental problem with promises as cancellation tokens is memory leaks. In your example, if the cancel button isn't clicked for 10 seconds, the token promise will reference 3 () => clearTimeout(id) callbacks which close over their respective ids. Three functions and three integer ids doesn't sound like much, but in real applications with long-running un-cancelled operations a token could accumulate quite an amount of resources which cannot be collected. A clever programmer might make the callbacks become cheap no-ops, but still at least the functions themselves will consume memory. For the simple programmer, we need an automatic (not error-prone) unsubscription mechanism once the respective cancellable operation ended.

Thanks for the links. I think I'm in the camp of not being concerned about this. Recall I'm not proposing new functionality, just using promises, so this stands to benefit from optimizations browsers ought to make already, without needing special attention. Once browsers optimize:

function poll() { return isDone() || wait(1000).then(poll); }

I'll worry about this. ;)

Yeah, I just think that we need new functionality (like the ability to remove callbacks from a promise) to solve cancellation properly.

It's true that ES6 has a bug that prevents implementors from optimising recursive assimilation, but it's a different kettle of fish to fix that in the spec. I'm trying to avoid that we make the same mistake again for cancellation tokens, so I think you should be concerned.

kind , Bergi

# Jan-Ivar Bruaroey (8 years ago)

On 10/28/16 8:39 AM, Bergi wrote:

Jan-Ivar Bruaroey wrote:

If you try the fiddle - jsfiddle.net/jib1/jz33qs32 - you'll see cancelling terminates the chain. If you intersperse non-cancellable operations, there'd be a delay if cancel is detected during those.

Yes, that's what I mean. Sure, I could use Promise.race to get the cancellation even if the non-cancellable operation resumes, but that's quite ugly:

Promise.race([ promise() …chain…, cancelToken ]).then(callback);

especially when you'll need to nest that pattern.

To be clear, the non-cancellable operation won't "resume" in the face of a CancelledError. Only if the cancel happened to trigger during one of the non-cancellable actions would there be a slight delay until that non-cancellable operation finished (which I consider a feature) and if a cancellable operation follows it, cancellation will happen at that point.

In someone can't tolerate that, then Promise.race is well-defined to do exactly what you show, and works in harmony with this pattern. Why reinvent the wheel?

And you'd Promise.race against the entire chain, so no need to nest this pattern typically. This is what I mean with focusing on the minimal use-case. Most people just want us to solve fetch already, so that expensive network resources can be freed. To get out of the current inertia, why not define:

 fetch (url, { cancelPromise: token })

now and use this pattern, and leave the more desirable { cancel } name for whatever future invention we hope will replace it (or annex it if nothing better materializes)?

.: Jan-Ivar :.

# Herby Vojčík (8 years ago)

Jan-Ivar Bruaroey wrote:

On 10/28/16 8:39 AM, Bergi wrote:

Jan-Ivar Bruaroey wrote:

If you try the fiddle - jsfiddle.net/jib1/jz33qs32 - you'll see cancelling terminates the chain. If you intersperse non-cancellable operations, there'd be a delay if cancel is detected during those.

Yes, that's what I mean. Sure, I could use Promise.race to get the cancellation even if the non-cancellable operation resumes, but that's quite ugly:

Promise.race([ promise() …chain…, cancelToken ]).then(callback);

especially when you'll need to nest that pattern.

To be clear, the non-cancellable operation won't "resume" in the face of a CancelledError. Only if the cancel happened to trigger during one of the non-cancellable actions would there be a slight delay until that non-cancellable operation finished (which I consider a feature) and if a cancellable operation follows it, cancellation will happen at that point.

In someone can't tolerate that, then Promise.race is well-defined to do exactly what you show, and works in harmony with this pattern. Why reinvent the wheel?

And you'd Promise.race against the entire chain, so no need to nest this pattern typically. This is what I mean with focusing on the minimal use-case. Most people just want us to solve fetch already, so that expensive network resources can be freed. To get out of the current inertia, why not define:

fetch (url, { cancelPromise: token })

OTOH, why not to just use Promise.race directly and promote the pattern of "specify alternate result".

  1. This is more general;
  2. This allows creating decorators and use them like shortcutAfter(5000, Promise.reject())(fetch(url))
# Herby Vojčík (8 years ago)

Herby Vojčík wrote:

Jan-Ivar Bruaroey wrote:

On 10/28/16 8:39 AM, Bergi wrote:

Jan-Ivar Bruaroey wrote:

If you try the fiddle - jsfiddle.net/jib1/jz33qs32 - you'll see cancelling terminates the chain. If you intersperse non-cancellable operations, there'd be a delay if cancel is detected during those.

Yes, that's what I mean. Sure, I could use Promise.race to get the cancellation even if the non-cancellable operation resumes, but that's quite ugly:

Promise.race([ promise() …chain…, cancelToken ]).then(callback);

especially when you'll need to nest that pattern.

To be clear, the non-cancellable operation won't "resume" in the face of a CancelledError. Only if the cancel happened to trigger during one of the non-cancellable actions would there be a slight delay until that non-cancellable operation finished (which I consider a feature) and if a cancellable operation follows it, cancellation will happen at that point.

In someone can't tolerate that, then Promise.race is well-defined to do exactly what you show, and works in harmony with this pattern. Why reinvent the wheel?

And you'd Promise.race against the entire chain, so no need to nest this pattern typically. This is what I mean with focusing on the minimal use-case. Most people just want us to solve fetch already, so that expensive network resources can be freed. To get out of the current inertia, why not define:

fetch (url, { cancelPromise: token })

OTOH, why not to just use Promise.race directly and promote the pattern of "specify alternate result".

  1. This is more general;
  2. This allows creating decorators and use them like shortcutAfter(5000, Promise.reject())(fetch(url))

Well, "shortCircuitAfter" would be probably better name.

# Jan-Ivar Bruaroey (8 years ago)

On 10/31/16 2:39 PM, Herby Vojčík wrote:

Jan-Ivar Bruaroey wrote:

On 10/28/16 8:39 AM, Bergi wrote:

Jan-Ivar Bruaroey wrote:

If you try the fiddle - jsfiddle.net/jib1/jz33qs32 - you'll see cancelling terminates the chain. If you intersperse non-cancellable operations, there'd be a delay if cancel is detected during those.

Yes, that's what I mean. Sure, I could use Promise.race to get the cancellation even if the non-cancellable operation resumes, but that's quite ugly:

Promise.race([ promise() …chain…, cancelToken ]).then(callback);

especially when you'll need to nest that pattern.

To be clear, the non-cancellable operation won't "resume" in the face of a CancelledError. Only if the cancel happened to trigger during one of the non-cancellable actions would there be a slight delay until that non-cancellable operation finished (which I consider a feature) and if a cancellable operation follows it, cancellation will happen at that point.

In someone can't tolerate that, then Promise.race is well-defined to do exactly what you show, and works in harmony with this pattern. Why reinvent the wheel?

And you'd Promise.race against the entire chain, so no need to nest this pattern typically. This is what I mean with focusing on the minimal use-case. Most people just want us to solve fetch already, so that expensive network resources can be freed. To get out of the current inertia, why not define:

fetch (url, { cancelPromise: token })

OTOH, why not to just use Promise.race directly and promote the pattern of "specify alternate result".

  1. This is more general;
  2. This allows creating decorators and use them like shortcutAfter(5000, Promise.reject())(fetch(url))

Because it doesn't make fetch stop fetching, which is what people want as I understand it (to not have the fetch keep going even though they've stopped waiting for it).

.: Jan-Ivar :.

# Jan-Ivar Bruaroey (8 years ago)

Happy New Year!

Here's a cleanup iteration on this proposal: jsfiddle.net/jib1/wyq4sxsc

You need Chrome or Firefox Developer Edition to run it due to async/await.

I've renamed the error to "CancelError" and made cancellation always fail with it, removing the "resumption" option.

PTAL!

Some comments on the earlier thread here:

  • The above fiddle suffers no accumulative "memory leak" problems. GC just works.
  • Synchronous inspection is necessary in multi-threaded cancellation only, not JS.

.: Jan-Ivar :.

# Igor Baklan (8 years ago)

In general I thing it would be good to have something like java(Thread.interrupt()), assuming that "Thread" here can be "async stacktrace of promises", and interrupt notification should be just forwarded to the top entry (promise-executor) and handled there arbitrary. In meta code it can be approximately expressed like promise.interrupt(interruption_config) ==> promise.topPromiseInAwaitChain().injectInterruptionNotification(interruption_config). So it should be up to promise-executor - whether it want to complete abnormally on interruption (either with success or with failure), or just ignore this signal and continue execution without any reaction. It just assumes that general .then(...)-like promises or async (...) => {...}-like promises should propagate interruption-signal transparently upward over async-stacktrace, and interruption-signal would be handled only in promises with executors which aware of interruption capability. In code it may look like

const waitAsync = (delay) => (new Promise(
  (resOk, resErr, interuptionSignalEmitter) => {
    const beforeComplete = () => {clearTimeout(tid);}
    const tid = setTimeout(
      () => {beforeComplete(); resOk();},
      delay
    );
    interuptionSignalEmitter.on(
      (interuptionConfig) => {
        beforeComplete();
        if (interuptionConfig && interuptionConfig.raiseError) {
          resErr(new Error("abnormal interuption"));
        } else {
          resOk();
        }
      }
    );
  }
));

// waitAsync(100).then(() => {console.log("wait ok")}).interrupt() - should complete successfully 
//   ("wait ok" message should be printed) but without delay in 100ms
// waitAsync(100).then(() => {console.log("wait ok")}).interrupt({raiseError: true}) - should complete with failure
//   (NO "wait ok" message should be printed) and without delay in 100ms

So, in other words, I would rather say, that we lack something like event capturing pahase when we intent for abnormal-completion/cancellation. I mean, if we have for example some async-stacktrace and some particular entry in it in the middle (some running and not yet completed promise), then it looks very natural that we may wish to "send a signal/message" downward over async-stacktrace, it just can be made by throwing something in that entry (and that "thrown something" will be naturally propagated downward async-stacktrace/promises-chain). But in certain cases we may need to "send a signal/message" upward over async-stacktrace which should generally end up by handling in very top promise executor (and if that top-most promise in chain decide to complete execution abnormally, then all clean-up in promises-chain also happens "abnormally"), while if we just "unsubscribe" some middle entry form it's "natural parent" and "abnormally" assign some result to that entry, then ofcourse, all cleanup in "upper part" of async-stacktrace will happen later (which ofcourse also can be desired behavior in some cases).

Jan-Ivar Bruaroey wrote:

Because it doesn't make fetch stop fetching, which is what people want as I understand it (to not have the fetch keep going even though they've stopped waiting for it).

Completely agree, but I would rather prefer

fetch(someUrl, someConfig).interuptOn(interruptionToken)

vs

fetch(someUrl, {...someConfig, cancel: interruptionToken})

in this case .interruptOn can be easily defined on top of .interrupt like promise.interruptOn(interruptionReasonToken) <==> interruptionReasonToken.then((reason) => {promise.interrupt(reason)}), promise

Bergi wrote:

Yes, that's what I mean. Sure, I could use Promise.race to get the cancellation even if the non-cancellable operation resumes, but that's quite ugly:

Promise.race([ promise() …chain…, cancelToken ]).then(callback);

especially when you'll need to nest that pattern. Instead, I'd just like to write

promise() …chain… .then(callback, cancelToken)

with the same behaviour.

Very reasonable. left-to-right . chaining is much more readable and concise then wrap-like expression chaining. But I would rather put cancelToken somewhere else, not in .then call. From my perspective it can look like

Promise.race([
     promise()
     …chain…,
     cancelToken
]).then(callback);

<==>

promise() …chain… cancellable().cancelOn(cancelToken).then(callback);

Where Promise::cancellable (Promise.prototype.cancellable) can return some sort of "wrapper" - CancellablePromise(targetPromise) which in turns can be implemented based on Promise.race([targetPromise, CancellablePromise::privateInternalCancelToken]) and that privateInternalCancelToken can be fully controlled by CancellablePromise itself and completed(fulfilled/rejected) when ever it needed (whether by direct call to cancellablePromise.completeNow(...) or caused by completion of some promise passed to .cancelOn method)

And finally

Bergi wrote:

I'd however love to be able to cancel specific chaining operations, i.e. then callbacks.

As for me, definitely such kind of possibility should be available. However we can not unsubscribe then-callback same easily as regular event handler. Since at least some sort of finally-callbacks should always work on any item in remaining promises chain. So promise then-unsubscription is tightly bound to providing immediate completion result for that target promise (which would be used instead of callback invocation result). And still it can be accomplished by some kind of third-party solutions, (like in snippets above) like promise.cancellable().then(() => {doSomth()}).completeImmediately({error:new Error("unsubscribed abnormally")}). If .cancellable() returns some wrapped instance like CancellablePromise(targetPromise) then ofcourse CancellablePromise can re-implement .then, .catch, etc by wrapping passed functions and by wrapping targetPromise.then(...wrappedCallbacks) with appropriate Promise.race([...]) like Promise.race([targetPromise.then(...wrappedCallbacks), internalCancellationToken]), and then when .completeImmediately invoked coordinate appropriately wrapped callbacks and internalCancellationToken to prevent initial callbacks from being executed and to complete "overall resulting promise" by the means of internalCancellationToken. But. I think disregarding that all of this stuff can be (more or less) implemented in 3-rd parity libraries, it would be much better if it was supported natively.

# Isiah Meadows (8 years ago)

Um... This isn't much different than Bluebird's Promise.prototype.cancel, admittedly the least optimal of all these so far.

# Igor Baklan (8 years ago)

Um... This isn't much different than Bluebird's Promise.prototype.cancel, admittedly the least optimal of all these so far.

Yes, very similar, in that it is propagated upward on "async-stacktrace", and can be actually handled in promise-executor. But very different in a way how callbacks of "affected" promises should be treated. As I can understand from bluebirdjs-cancellation article - on cancellation method invocation it always go into cancellation action (which is delivered on that "3-rd rail", so no success neither failure callbacks are invoked but consistency still preserved - finally callbacks still executed - which is really nice option but ...). But what I say in this "interruption" idea, that it should be up to promise-executor what action should be taken on underlying promise (and in turn propagated downward async-stacktrace). So that when you call here somePromice.interrupt(someInterruptionConfig) it may end up with any kind of results - success/failure/3rd-rail-cancellation - and it should be specified in particular promise-executor (generally in top-most promise in async-stacktrace) how to react on this particular someInterruptionConfig passed into .interrupt(...) method. However I should admit that most likely "implementers" and "users" of such functionality would prefer and intent exactly that behavior from bluebirdjs-cancellation , and would rather want always implement that kind of behavior by default, but what I actually don't like from that - is that decision "how to complete underlying/topmost-interrupted promise" is taking without "asking promise-executor what it thinks about it before(not after) canceling" (whether it "objects" this abrupt execution, maybe it prefers to continue working and just ignore this interruption signal, or may be it already has some default success/failure result and would prefer to complete abruptly but with its own default result etc).

So again what is not very good with bluebirdjs-cancellation, that promise-executor can not intercept this signal, and can not "preventDefault"-behavior, but instead it is only notified that "everything(cancellation) already happened" (as I can conclude from bluebirdjs-cancellation)

And finally, I think it is also very nice to have that cancellation with "firm contracts" - which always/unavoidably cancels promises chain (for example by the means of that 3-rd-cancellation-rail). But as for me it would be also good to have that more "soft" functionality which delegates to promise-executor decision on which "rail" (success/failure/cancellation) and with which actual "value" deliver that abnormal execution completion / cancellation.

# Igor Baklan (8 years ago)

Check on practice how bluebirdjs-cancellation works and find out that promise-executor.onCancel callback get executed before finally blocks of all affected promises, so potential api for overriding default cancellation delivery mechanism/"rail" (success/failure/cancellation) might exist in lib still however I can not see them in bluebirdjs documentation.

# Jan-Ivar Bruaroey (8 years ago)

Cancellable promises is dead. Please don't hijack this thread discussing them.

Thanks,

.: Jan-Ivar :.

# Jordan Harband (8 years ago)

The Cancellable Promises proposal itself is currently withdrawn, but don't forget that all of the previous discussion on cancellation in promises led to that proposal.

It would be shortsighted to pretend they don't exist, or that the spec proposal won't matter forever to any other cancellation proposal, and doing so won't help any alternative proposal.

# Isiah Meadows (8 years ago)

And that's why we're waiting on the next meeting to happen with notes posted, so we can figure out what to do next. It's likely to get discussed, especially considering the current situation and pressing need for it.

# /#!/JoePea (7 years ago)

Hello all, I'm posting here because I'm thinking about how not to leak memory, and wondering if I am.

Suppose I have an object that is cleaned up (GC'd), and that object's constructor (for example) ran an anonymous function that listened to a Promise. For exampel:

class Foo {
  constructor() {
    // ...
    this.somePromise.then(() => {console.log('this will never fire if this

object is GC'd')})
  }
}

Suppose that if this object gets collected, then it means that there is nothing in my application that will ever be able to resolve this object's somePromise.

Does the engine also garbage collect somePromise and the attached handler?

Now, what if somePromise is referenced outside of this object by some other code waiting on yet another promise who's handler will resolve this object's somePromise? Does this prevent the object from being GC'd? Or is the object GC'd, but somePromise is not and the above console.log should still fire at some point (or memory will be leaked)?

# Isiah Meadows (7 years ago)

On Wed, Nov 29, 2017, 19:19 /#!/JoePea <joe at trusktr.io> wrote:

Hello all, I'm posting here because I'm thinking about how not to leak memory, and wondering if I am.

Suppose I have an object that is cleaned up (GC'd), and that object's constructor (for example) ran an anonymous function that listened to a Promise. For exampel:

class Foo {
  constructor() {
    // ...
    this.somePromise.then(() => {console.log('this will never fire if this
object is GC'd')})
  }
}

Suppose that if this object gets collected, then it means that there is nothing in my application that will ever be able to resolve this object's somePromise.

Does the engine also garbage collect somePromise and the attached handler?

Yes, provided nothing else is referencing it.

Now, what if somePromise is referenced outside of this object by some other code waiting on yet another promise who's handler will resolve this object's somePromise? Does this prevent the object from being GC'd? Or is the object GC'd, but somePromise is not and the above console.log should still fire at some point (or memory will be leaked)?

The object itself is GC'd independently of its members absent a circular reference.

A thing to keep in mind is that a promise could still be referenced by its resolving functions, even if the promise itself is no longer referenced by ECMAScript code. This keeps the promise state alive, so it can still fire (otherwise, you'd have observable GC). So, in this example, things will still fire regardless:

function foo() {
    let p = new Promise(resolve => {
        setTimeout(resolve, 1000)
    })

    p.then(() => {
        console.log("This still fires")
    })

    // `p` is no longer referenced by the
    // calling function, but it's still referenced
    // by the enqueued `setTimeout` task, so
    // it can't be collected
}

foo()
# kai zhu (7 years ago)

@joe, its generally bad-practice and confusing to user, to include async initialization code inside a constructor. you are better off using a class-factory with some kind of async callback-mechanism, which at the very least informs the user you’re doing an async-operation during construction (and preventing any gc while async-code inside the factory-function is still active).

here's a real-world example of using a class-factory with callback, to asynchronously “construct” a persistent db-table from indexeddb

kaizhu256.github.io/node-db-lite/build..beta..travis-ci.org/apidoc.html#apidoc.element.db-lite.dbTableCreateOne, kaizhu256.github.io/node-db-lite/build..beta..travis-ci.org/apidoc.html#apidoc.element.db-lite.dbTableCreateOne

dbTableCreateOne = function (options, onError) { /*

  • this function will create a dbTable with the given options / var self; options = local.normalizeValue('dict', options); // register dbTable self = local.dbTableDict[options.name] = local.dbTableDict[options.name] || new local._DbTable(options); self.sortDefault = options.sortDefault || self.sortDefault || [{ fieldName: '_timeUpdated', isDescending: true }]; // remove idIndex local.normalizeValue('list', options.idIndexRemoveList).forEach(function (idIndex) { self.idIndexRemove(idIndex); }); // create idIndex local.normalizeValue('list', options.idIndexCreateList).forEach(function (idIndex) { self.idIndexCreate(idIndex); }); // upsert dbRow self.crudSetManyById(options.dbRowList); self.isLoaded = self.isLoaded || options.isLoaded; if (!self.isLoaded) { // asynchronously load persistent dbTable from indexeddb if it exists local.storageGetItem('dbTable.' + self.name + '.json', function (error, data) { // validate no error occurred local.assert(!error, error); if (!self.isLoaded) { local.dbImport(data); } self.isLoaded = true; local.setTimeoutOnError(onError, null, self); }); return self; } return local.setTimeoutOnError(onError, null, self); } storageGetItem = function (key, onError) { /
  • this function will get the item with the given key from storage / defer({ action: 'getItem', key: key }, onError); } storageDefer = function (options, onError) { /
  • this function will defer options.action until storage is ready / var data, isDone, objectStore, onError2, request, tmp; onError = onError || function (error) { // validate no error occurred local.assert(!error, error); }; if (!storage) { deferList.push(function () { defer(options, onError); }); init(); return; } switch (modeJs) { case 'browser': onError2 = function () { / istanbul ignore next / if (isDone) { return; } isDone = true; onError( request && (request.error || request.transaction.error), data || request.result || '' ); }; switch (options.action) { case 'clear': case 'removeItem': case 'setItem': objectStore = storage .transaction(storageDir, 'readwrite') .objectStore(storageDir); break; default: objectStore = storage .transaction(storageDir, 'readonly') .objectStore(storageDir); } switch (options.action) { case 'clear': request = objectStore.clear(); break; case 'getItem': request = objectStore.get(String(options.key)); break; case 'keys': data = []; request = objectStore.openCursor(); request.onsuccess = function () { if (!request.result) { onError2(); return; } data.push(request.result.key); request.result.continue(); }; break; case 'length': request = objectStore.count(); break; case 'removeItem': request = objectStore.delete(String(options.key)); break; case 'setItem': request = objectStore.put(options.value, String(options.key)); break; } ['onabort', 'onerror', 'onsuccess'].forEach(function (handler) { request[handler] = request[handler] || onError2; }); // debug request local._debugStorageRequest = request; break; case 'node': switch (options.action) { case 'clear': child_process.spawnSync('rm -f ' + storage + '/', { shell: true, stdio: ['ignore', 1, 2] }); setTimeout(onError); break; case 'getItem': fs.readFile( storage + '/' + encodeURIComponent(String(options.key)), 'utf8', // ignore error function (error, data) { onError(error && null, data || ''); } ); break; case 'keys': fs.readdir(storage, function (error, data) { onError(error, data && data.map(decodeURIComponent)); }); break; case 'length': fs.readdir(storage, function (error, data) { onError(error, data && data.length); }); break; case 'removeItem': fs.unlink( storage + '/' + encodeURIComponent(String(options.key)), // ignore error function () { onError(); } ); break; case 'setItem': tmp = os.tmpdir() + '/' + Date.now() + Math.random(); // save to tmp fs.writeFile(tmp, options.value, function (error) { // validate no error occurred local.assert(!error, error); // rename tmp to key fs.rename( tmp, storage + '/' + encodeURIComponent(String(options.key)), onError ); }); ... storageInit = function () { /*
  • this function will init storage */ var onError, request; onError = function (error) { // validate no error occurred local.assert(!error, error); if (modeJs === 'browser') { storage = window[storageDir]; } while (deferList.length) { deferList.shift()(); } }; if (modeJs === 'browser') { storage = window[storageDir]; } if (storage) { onError(); return; } switch (modeJs) { case 'browser': // init indexedDB try { request = window.indexedDB.open(storageDir); // debug request local._debugStorageRequestIndexedDB = request; request.onerror = onError; request.onsuccess = function () { window[storageDir] = request.result; onError(); }; request.onupgradeneeded = function () { if (!request.result.objectStoreNames.contains(storageDir)) { request.result.createObjectStore(storageDir); } }; } catch (ignore) { } break; case 'node': // mkdirp storage storage = storageDir; child_process.spawnSync( 'mkdir', ['-p', storage], { stdio: ['ignore', 1, 2] } ); onError(); break; } }
# Naveen Chawla (7 years ago)

kai, 1. too much code to illustrate your point, 2. It's as much bad practice to kick off async functions from a constructor as it is to kick them off from any synchronous function. i.e., not at all. 3. I think these GC worries are over the top. Yes, if you have promises being awaited that never resolve, they will build up if you keep creating them. In this case, the solution can perhaps be a timeout that rejects or resolves them. That's it. I'm not sure this issue is any more complex than this. Somebody point me to an example case that shows otherwise if you think I'm wrong. Something isn't garbage collected if it's active, otherwise it is eventually. Relax.