-
Notifications
You must be signed in to change notification settings - Fork 64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid an indefinite recursion that grows the call stack when reporting the current state fails #480
Conversation
@pcarranzav I'd be surprised if that |
@Page- this seems to be the only possible culprit - maybe related to petkaantonov/bluebird#1326 - my expectation is that this promise chain will be resolved and a new one will be created, clearing the stack |
(Still not sure though, investigating) |
I just ran this, left it running overnight, and woke up to an "out of memory" error:
(on a resin/amd64-node:6.5-slim container) |
@Page- there seems to be a few issues in bluebird relating to the stack limit being reached in similar loops, which makes me think there must be some scenario in which such an async construct doesn't work as expected |
215cfc0
to
b0bb3b5
Compare
Also @Page-, as per this comment, storing the returned promise is an antipattern that can cause a bit of a memory bloat (it should be cleared after 4 interactions, but still...). |
b0bb3b5
to
2680aa3
Compare
…g the current state fails We used to have a recursion based on Promises and Promise.delay, which caused the promise never to resolve so eventually the stack would be exhausted. This fixes it by using a simpler way to check if reporting the state is in progress and using a setImmediate to call applyState outside of the Promise chain. Change-Type: patch Signed-off-by: Pablo Carranza Velez <pablo@resin.io>
2680aa3
to
51d6ab0
Compare
@pcarranzav, status checks have failed for this PR. Please make appropriate changes and recommit. |
We used to have a recursion based on Promises and Promise.delay, which caused the promise never to resolve
so eventually the stack would be exhausted.
This fixes it by using a simpler way to check if reporting the state is in progress and using a setImmediate to
call applyState outside of the Promise chain.
Change-Type: patch
Signed-off-by: Pablo Carranza Velez pablo@resin.io
Connects to https://github.com/resin-io/hq/issues/971
Front conversations