Fix OOM crash, when closing a transaction while Queries are still ongoing #1193
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ng queries I noticed that while closing a transaction and concurrently still having queries left running the driver eventually causes an OOM node error. This test case causes this exact error. A fix will (hopefully) come in the next commit
…ll running queries. When closing a transaction, that still had queries open the `_onErrorCallback` in core/src/transaction.ts:303 called `resultStreamObserver.onError` for each result. The default behaviour for `FailedObvserver` (one implementation) is to call `this._beforeError`(core/src/internal/observer.ts:179), which is set to `onError`, which happens to be set to the mentioned `_onErrorCallback`. This results in an infinite async-loop, which gradualy consumes all available memory and crashes the process. Because the `FAILED` state is only set in `_onErrorCallback` we can use it as a flag, to cut off the infinite recursion, when we are allready in the `FAILED` state.
|
@bigmontz (Sorry for the ping, but this is a server-crashing issue for us, which actually causes errors for our users.). If there is anything I can do to get this merged faster, please let me know :) Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When closing a transaction while queries are still running, the
_onErrorCallbackswill cause an infinite loop which leads to an OOM crash.Because
FAILEDis only set as a state in this function, I opted to use it as a failsafe check, to cut the recursion short.The test triggers the OOM crash when the fix is not included, otherwise it passes in <1s on my machine.
I hope the test is ok as it is, let me know if you need any changes for this.
EDIT: Upon a bit further testing, it's not actually infinite, but just scaling exponentially.
3 simultaneous queries gets us ~3000
_onErrorCallbackcalls,4 gets us ~32000,
5 gets us ~195000,
6 gets us ~840000,
7-12 gets us "Map maximum size exceeded"
13+ gets us the mentioned OOM