Skip to content

Instantly share code, notes, and snippets.

@v0lkan
Created January 19, 2019 06:13
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save v0lkan/cde36a9d9c56b4b96a52d1f2303b06f2 to your computer and use it in GitHub Desktop.
Save v0lkan/cde36a9d9c56b4b96a52d1f2303b06f2 to your computer and use it in GitHub Desktop.
Promise Loop
/*
* \
* \\,
* \\\,^,.,,. JavaScript: from Zero to Hero
* ,;7~((\))`;;,, <zerotoherojs.com>
* ,(@') ;)`))\;;', an extraordinary course to learn JavaScript
* ) . ),(( ))\;, and related technologies
* /;`,,/7),)) )) )\,,
* (& )` (,((,((;( ))\,
*
*/
// Creating a promise chain is not strictly recursive because each
// `then(fn)`’s `fn` is executed in its own stack.
//
// HOWEVER; the resulting promise will need to maintain a chain of
// resolutions, so that once (if) the promise chain eventually resolves,
// it will pass the resolution value all the way up to the first promise.
//
// That’s why, if you don’t limit the depth of your promise chains you
// may leak memory and even crash the entire Node.js process if there’s
// not enough heap space left for Node.js to consume.
//
// Here is a one-liner to exhaust your Node.js app and crash it.
const loop = async () => Promise.resolve(true).then(loop);
loop();
// Terminal output will be similar to this:
//
//
// <--- Last few GCs --->
//
// [31436:0x103800c00] 17448 ms: Scavenge 1388.2 (1423.2) -> 1387.8 (1423.7) MB, 1.9 / 0.0 ms (average mu = 0.162, current mu = 0.129) allocation failure
// [31436:0x103800c00] 17454 ms: Scavenge 1388.5 (1423.7) -> 1388.1 (1424.2) MB, 5.2 / 0.0 ms (average mu = 0.162, current mu = 0.129) allocation failure
// [31436:0x103800c00] 17456 ms: Scavenge 1388.8 (1424.2) -> 1388.4 (1425.2) MB, 2.3 / 0.0 ms (average mu = 0.162, current mu = 0.129) allocation failure
//
//
// <--- JS stacktrace --->
//
// ==== JS stack trace =========================================
//
// 0: ExitFrame [pc: 0x3113334fb7d]
// 1: StubFrame [pc: 0x3113333e7f8]
// 2: StubFrame [pc: 0x3113331c6d2]
// 3: EntryFrame [pc: 0x31133305c9e]
// 4: ExitFrame [pc: 0x3113334fb7d]
// Security context: 0x2e871991d969 <JSObject>
// 5: _tickCallback [0x2e87f5993b49] [internal/process/next_tick.js:43] [bytecode=0x2e875eb3c269 offset=49](this=0x2e87827823d9 <process map = 0x2e875fccc941>)
// 6: /* anonymous */ [0x2e87bb559791] [internal/m...
//
// FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
// 1: 0x10003a9d9 node::Abort() [/usr/local/bin/node]
// 2: 0x10003abe4 node::FatalTryCatch::~FatalTryCatch() [/usr/local/bin/node]
// 3: 0x10019ed17 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/usr/local/bin/node]
// 4: 0x10019ecb4 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/usr/local/bin/node]
// 5: 0x1005a5882 v8::internal::Heap::FatalProcessOutOfMemory(char const*) [/usr/local/bin/node]
// 6: 0x1005a4838 v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) [/usr/local/bin/node]
// 7: 0x1005a2443 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/usr/local/bin/node]
// 8: 0x1005aecbc v8::internal::Heap::AllocateRawWithLightRetry(int, v8::internal::AllocationSpace, v8::internal::AllocationAlignment) [/usr/local/bin/node]
// 9: 0x1005aed3f v8::internal::Heap::AllocateRawWithRetryOrFail(int, v8::internal::AllocationSpace, v8::internal::AllocationAlignment) [/usr/local/bin/node]
// 10: 0x10057dfc4 v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationSpace) [/usr/local/bin/node]
// 11: 0x100832070 v8::internal::Runtime_AllocateInNewSpace(int, v8::internal::Object**, v8::internal::Isolate*) [/usr/local/bin/node]
// 12: 0x3113334fb7d
// 13: 0x3113333e7f8
// 14: 0x3113331c6d2
@joeyhub
Copy link

joeyhub commented Oct 19, 2019

I don't really understand the problem. Infinite loops are well known. while(1); will hang your app. There might be a problem with users being fooled into a false sense of security by many engines having a low limit on the stack depth.

I don't see where you're getting at with memory leaks. I believe you have have inadvertently stumbled on something though.

I assume JS does something to stop leaks with something like when you have:

const loop = function() {
    setTimeout(loop, 1000);
};

loop();

If not that's a problem, all events chain back to an original event which would make an infinite stack. The magic that does to run on its own stack should be a function in JS that's not embedded in promises (usually such a function might return a promise though). Unfortunately promises in JS are completely FUBAR.

Something that is problematic, you may then get leaks with developer tools on but not off, it can be hard to test and to be sure about things.

Your example never actually goes onto the event loop and never truly breaks the chain of execution and nor should it as it is synchronous code.

Promises in JS follow a defective design (Promise/A+) which has been mindlessly fudged over time into some weird half one thing half another. It has its own immediate queue where it pretends to run async but isn't really. That's a good candidate for unexpected differences in behaviour.

There are probably lots of fun ways to kill the process like this. Just push forever into an array. Try some string repeat or some thing like Array(hugenumber). What happens if you flatten something with an array with a circular reference?

I believe if you fiddle around you can turn the stack depth up or off then achieve the same with:

(f => f(f))(f => f(f))

When you consider that promises aren't meant to run async (it's nothing to do with what it means for something to be a promise) at all and assume they're meant to run sync that Promise/A+ was never designed for async functions or never designed particularly well at all then your confusion might go away.

Another point to the confusion this design mistake makes is that Promise/A+ might have a minimal primitive guard against the most immediate infinite loop case.

In your case both the guards are off and also because then does this weird fake async you can get confused because you assume it should be truncating the stack like true async does and not stack overflowing but exactly what promises do, you can't really know for sure, they smush together a bunch of concerns as well as hidden behaviours that go beyond the utility of promises meaning you can't use common sense.

The point of the async/await implementation is to revert back to normal functional behaviour which means things such as keeping the stack for example for exceptions. All those async instances will still be waiting around to receive catch events wont they? I think I have problems like this with my event implementations sometimes where I use destroy, destruct or weakrefs (something like this has a chase the tail dynamic usually). I don't think you can solve this with weakrefs. I may be mistaken since it's not awaiting.

Something interesting, see what happens when you make a process that runs itself (possibly through more than one hop) or a cgi script that requests itself again from localhost. Linux often handles those situations really badly.

When it comes to Promise/A+ and Zalgo, forget everything you learnt. That's some group think stuff and posing, it's not real compsci or real programming, instead better to just play around and figure stuff out yourself.

const loop = async () => Promise.resolve(true).then(loop);
loop();

Is potentially the same as:

const f => new Promise(resolve => {
    resolve((new Promise(resolve => resolve())).then(f));
});

It should be able to GC each function afterwards in theory. Loop does return a promise but it should fall into thin air as the promise has nothing to do with it (no chain).

const loop = async () => Promise.resolve().then(async () => Promise.resolve().then(async () => Promise.resolve().then(loop)));

This is weird though because that invocation will return a promise that returns a promise then that promise. I can't help suspecting there's some accidental internal chaining happening there given it seems like every promise would again yield another promise eternally?

Personally I've been avoiding returning promises from async/functions due to such confusion scared of what might end up happening. Perhaps I should play with that more.

a = Promise.resolve(b = Promise.resolve())
a === b
true
a = new Promise(r => r(b = Promise.resolve()))
a === b
false
c = (a = Promise.resolve()).then(b = Promise.resolve())
a === b
false
a === c
false
b === c
false

Isn't the promise c appended to b as a then? I feel daft figuring this out given it turns out there's an explanation at the top.

@v0lkan
Copy link
Author

v0lkan commented Oct 19, 2019

Thanks for your comments Joey.

The then() in a promise chain is always async.

console.log(1);
Promise.resolve().then(function () {
  console.log(3);
});
console.log(2);
// will log 1 2 3 

And by extension, any code you chain with then() is asynchronous.

The code never going to the event loop can be the problem, because it builds up a resolution tree (but not an execution stack) in memory ad infinitum.

This following code, for example,

const loop = () => Promise.resolve(true).then(
  () => new Promise(
    (resolve) => 
      setImmediate( () => resolve(loop()) )  
  )
)

loop()

Is certainly async (due to setImmedaite) and it will also slowly consume more and more memory (while the CPU utilization will remain more or less constant.

Honestly, it has been a while, and I don’t remember if I found an actual memory leak, or whether I was stating the obvious, or whether this was a draft for a zerotoherojs.com lesson.

But I can say that the promise chain in the example does not overfllow the execution stack, they overflow memory.
Because it needs to store the resolution list (tree/graph/or-whichever internal structure the engine has) so that it can resolve them later (typically at next tick, if it has a chance) in order.

The example here is different than a while { fn() } or function fn() { fn()}

The stack-like structure thats being built up is not a call-stack, so it is not bound by JavaScript runtimes max stack size limitation.
Its rather bound by Node.js engines ~1G max memory limitation.

I mostly agree with your sentiments though :)

@joeyhub
Copy link

joeyhub commented Oct 19, 2019

I figured in the end you're right though there are a lot of details here. It wasn't obvious when just jumping to the code and assuming the opening comments were noise. At first it looks like a normal infinite loop.

There is a lot of scope for confusion around what's meant by async. Async is a concept with an approximate or rough definition but it becomes specific with certain implementations. With promises the implementation of then's async is a contrivance and it's a different implementation to how async traditionally works in JS. This has the following result:

  • It can be hard to say under what meaning of async it qualifies.
  • It cannot be assumed to necessarily exhibit the same properties as other implementations.

It's a kind of pseudo async, you can technically simulate what it does fully with synchronous code with some technicality over what is happening with the stack. However, if it states that it truncates the stack then it should do so and should not be expected to overflow in any manner.

The promise chain doesn't overflow the stack though it is being used with the intention of restoring the function stack with async/await which it doesn't do properly because it's a queue rather than a stack making another area of confusion. It approximates asynchronicity and it approximates callstacks with unspecified behaviour in many cases. Pseudo callstacks and pseudo async.

Your example here shows yet a further peculiarity arising from basing async functions on Promise/A+. The chaining makes it like having a hidden await which has the effect of creating a function stack in the form of promise chains / a tree of listeners. With async functions the idiosyncrasies of Promise/A+ tend to be revealed.

I believe you're ending up with something approaching this:

loop = async () => (async () => await loop())()

Since then and await are effectively the same.

I don't think you found an unambiguous memory leak but you found the way in which async is really just a shorthand for using promises in an unconventional way rather than straight up functions that can do something that's prone to lead to inadvertent memory leaks. As said in all my code I've been avoiding returning promises like this because I don't trust what happens when they meld which is different to returning a straight value. Whenever I have an async function immediately returning a promise I make it a sync function and never have any more promise nesting than needed or expected. I don't think I ever return anything I don't intend to return. All my arrow functions are void if I didn't intend to return something.

Though to be certain, I would try this out with a sync loop function. I have a feeling the same will happen with:

Promise.resolve().then(() => Promise.resolve().then(() => Promise.resolve().then(etc));

I think in my promise implementations and specifications when I put chain as false (false by default in my implementation as things should never do things unless asked to) it stops that though it was five years or more since I made that. I think dancing between weak refs and strong refs would also cover some cases but in a convoluted way.

I wouldn't entirely disqualify this from being a leak because I believe even with the chain there's stuff left only reffed in the chain that could be cleared up. To be more specific, you have items unreferenced except within the chain that are also unreachable within the chain. That makes it a candidate for collection. A more subtle suboptimal case arises where A..Z can be reduced to (A, Z). Truncating is normal but chopping off the head and tail then joining them is harder.

I'd also test with this:

const loop = async () => {Promise.resolve(true).then(loop)};

You might find this interesting.

@v0lkan
Copy link
Author

v0lkan commented Oct 21, 2019

Thanks for the link. I’ll check it out.

Nope I don’t claim I’ve found an unambiguous memory leak.

If anything, it is a more-or-less obvious logical leak.

So it depends on the definition of a leak; but from a practical standpoint it sure is leaking and crashing the system ;)

A non-strict definition of a leak can be: Anything that increases memory utilization without bound, and will likely crash or starve the system if not kept in check.

Cheers,

V.

@joeyhub
Copy link

joeyhub commented Oct 21, 2019

If I get the time later I'll see if I can make a minimal promise implementation that can truncate using weakrefs or see if it would need destructors.

If I can get it to collect that will answer the question less ambiguously.

@joeyhub
Copy link

joeyhub commented Oct 21, 2019

"use strict";
var queue = [];

let l = 0;

function LeakproofPromise() {
    var tooSoon, result;

    if(++l % 1000000 === 0)
        console.log(`L: ${l} ${d} ${i}`);

    this.resolve = (...arg) => {
        if(arg.length > 1 || result)
            throw new Error();

        result = arg;

        if(tooSoon) {
            queue.push(...tooSoon.map(tooSoon => [tooSoon, arg]));
            tooSoon = undefined;
        }
    };

    this.then = (callback) => {
        const child = new LeakproofPromise();

        if(result)
            queue.push([[callback, child], result]);
        else {
            if(!tooSoon)
                tooSoon = [];

            tooSoon.push([callback, child]);
        }

        return child;
    };
}

let d = 0;

const f = () => {
    if(++d % 1000000 === 0)
        console.log(`D: ${l} ${d} ${i}`);

    const a = new LeakproofPromise(), b = new LeakproofPromise();
    b.resolve();
    const c = b.then(f);
    a.resolve(c);
    return a;
};

(() => {
    const p = new LeakproofPromise();
    p.resolve();
    p.then(f);
})();

const stack = [];

const toStack = () => {
    if(!queue.length)
        return;

    //stack.push(...queue.reverse());
    stack.push(...queue);
    queue = [];
};

toStack();

let i = 0;

while(stack.length) {
    if(++i % 1000000 === 0)
        console.log(`I: ${l} ${d} ${i}`);

    //const [callback, arg] = stack.pop();
    const [[callback, child], arg] = stack.shift(), result = callback(...arg);

    if(result)
        result.then(child.resolve);
    else
        child.resolve(result);

    toStack();        
}

Just a straight up basic promise implementation is smacking up the CPU core that drew the short straw something good for me but with constant memory. I am sure the heads (new Promise) can be collected.

Need to confirm this promise implementation doesn't have some other loop bug though or some logic fail.

I wonder about catch though. I'm finding it not so easy surprisingly to get it to leak.

Confirmed as another design flaw in Promise/A+: nodejs/node#6673

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment