Fix recursion issue #20
Conversation
Creating a heapdump forces a garbage collection, which can then trigger the OOM flow again, which then calls OnOOMError which then creates a heapdump which forces GC which triggers OOM which... This recursive loop is bad for several reasons, not the least of which being if addTimestamp is true it will keep appending the timestamp to the filename, even past the end of the allotted 256 chars. Rather than let this happen let's instead treat it as an error condition and bail.
As noted in the last commit, capturing a heap snapshot forces a garbage collection which can recursively trigger the OOM flow. To prevent this we now raise the heap size limit while capturing this last dump. As far as I can tell this is safe, even in memory-constrained environments, because by the time we get to this section the JS execution has already been halted, so the JS heap won't grow any further.
|
Thanks for the PR, i missed it apparently. So not sure what to do with this PR. Any ideas? See https://github.com/blueconic/node-oom-heapdump/tree/feature/3.0.0 |
|
@simon-abbott i tried your PR out, and it works perfect on Node 16. I'll work on it some work (i want to update some deprecated libraries) and that works out, i'll publish a new version to NPM. |
|
@simon-abbott i published a new (tagged alpha) version 3.0.0 here: https://www.npmjs.com/package/node-oom-heapdump/v/3.0.0. You can test it by using I'm gonna test a bit more myself and also #22 need to be fixed still. |
Yes it technically does, but when I was testing it last it didn't work as well as I needed it to. I don't remember the exact problem(s) I ran into, but it definitely didn't check all the boxes for my use-case. This package does, however, so I'm glad to see it's still being at least lightly maintained. |
Creating a heapdump forces a garbage collection which can then trigger the OOM flow which then calls
OnOOMErrorwhich then creates a heapdump which then forces GC which then triggers the OOM flow which...This recursive loop is bad for several reasons, not the least of which being if addTimestamp is true it will keep appending the timestamp to the filename, even past the end of the allotted 256 chars. This PR solves this in two parts:
OnOOMErroris called more than once, just in case #1 didn't work.