-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Don't catch OutOfMemoryError #1699
Comments
I think those were implemented with a good intention in mind:
Thoughts? |
For However, for If every library was wrapping |
the argument against catching an |
@wind57 |
Unfortunately Gson is not really able to detect whether it caused the OOM itself or the OOM is caused by non-Gson stuff you said it yourself. I hear your reasons, I understand them; but I do not agree with them. Will see where this goes... thx. |
@wind57 |
Catching |
Gson currently catches
OutOfMemoryError
s at two places and wraps them insideJsonParseException
:gson/gson/src/main/java/com/google/gson/JsonParser.java
Lines 88 to 89 in c5a3f21
gson/gson/src/main/java/com/google/gson/JsonStreamParser.java
Lines 90 to 91 in b75e1bb
This is bad practice because Gson is not necessarily the cause for this error and wrapping it inside an
Exception
subclass prevents the caller from noticing theOutOfMemoryError
until later.Edit: Catching
OutOfMemoryError
might be desired to protect about malicious JSON data, see also #1912 (comment). However, it appearsOutOfMemoryError
is not caught consistently everywhere.The text was updated successfully, but these errors were encountered: