memory leaks

[geyang]

@fluffynuts thanks for this great module!

I looked through the source code and am wondering, the retaining of the history array in the global seems to be a potential source of memory leaks. There are a few tests we can do to show whether an unattached promise object gets GC'ed after a while. A quick look at the source code make it seem otherwise.

I haven't had the time to run some real test yet, but what's your take on this?

Thanks again, and Happy Holidays!

[geyang]

Also it looks like the module.history and SynchronousPromise._getHistory method are both not used. I figure you might use them for your tests.

This is currently the highest ranked (by usage) synchronous promise module on npm. Would you be interested in removing this history variable from the module? If so, I can submit a PR for this change.

[fluffynuts]

I'm happy to accept (and have released 2.0.4 with the merge).

The history collection was something I was using whilst trying to debug global usage of SynchronousPromise (replacing the global Promise) whilst using TypeScript. I wasn't aware that the memory usage of the history tracking was large enough to affect anyone -- SynchronousPromise is usually used during tests where even a couple hundred KB shouldn't matter -- the test process exits within a minute or two.

For more info on the why:
It turns out that TypeScript generates it's __awaiter assuming that:

1. The global Promise will never be hijacked (ie, they never capture it)
2. The global Promise will be asynchronous (so `SynchronousPromise causes async/await code to hang)

I've raised an issue (microsoft/TypeScript#19909) in which respondents move between not understanding the problem as stated (so I re-stated) and then claiming that TypeScript users should not be "affected" because of my "non-spec compliant" promise implementation. I've tried to supply a work-around, monkey-patching their __awaiter -- it works in tests, but, for some reason, still doesn't work in the one project where we were using this. The older SynchronousPromise does -- but only because of a bug in its behavior.

TL;DR: I don't recommend global installation of SynchronousPromise (ie, somewhere setting window.Promise or global.Promise to SynchronousPromise) within an environment which makes unprotected use of the global Promise object whilst assuming that it's asynchronous (eg, within TypeScript projects).

[geyang]

oh cool!

Thanks for explaining the rationale, after reviewing your comments in the other thread I think I understand the situation now. Also thanks for your quick release!

I have seem it before that the ES7 async function will return the default Promise. Although this does take away the freedom of using your own Promise, I can try to live with that. In their case the idea is that b/c the return type can not be injected to be anything else, the promise object also has to be the original one. I'm 50/50 on this.

I don't currently use this as global. My usage case involve using this only in place of the system promise with explicit import.

[fluffynuts]

Yeah, I think es7 is doing the right thing, especially if the underlying generator code relies on asynchronicity, as the TypeScript code does. Otherwise (and thus was my argument at the TypeScript issue thread) third-party code can break the defined behaviour of the system. I guess this problem will go away when es7 is adopted by browsers for their runtimes - at which point, TypeScript should have an option to use the built-in functionality instead of their breakable one 😒

Mostly, I really enjoy TypeScript - this one issue just annoys me 😔

[geyang]

haha I have to say the other thread is quite a bit confusing. If you want to push on it, maybe try to start a new thread (after a while) and make it super clear? like show it to friends. That thread is already confusing enough with all the comments so it is not necessarily bad that it is closed by the bot in the end. I think writing simple failure cases and pushing for changes is very important contribution :)

[geyang]

@fluffynuts btw, here is the library where I'm using this promise library!

https://github.com/escherpad/luna-saga

[fluffynuts]

I'm assuming you're using synchronous promise for testing? Using it for production code is obviously not a great idea 😊 I haven't done any react, though I think that may change this year. Last year, I had the misfortune to work with Polymer for the year. Spoiler alert: it's crap.