Fix JsonGenerator.close regression
Opened this issue · 3 comments
Please check #156, this issue introduced by a too quick review a big regression on JSON-P API design (and implementations/doc bugs) so it is more than likely and welcomed to revert this by documenting properly the behavior and harnessing it instead of breaking the JSON-P api design.
Actually I don't think it should close the underlying Writer/OutputStream at all.
The JsonGenerator isn't the owner of the stream and should never close it, on exception or not.
It's easy to leak resources when the code is asymmetric and it's also very hard to track/debug.
My golden rules
Don't delegate the closing of your resources to someone else.
Don't close someone's else resources.
The caller passes in the OutputStream or the Writer when creating the JsonGenerator, so the caller is responsible for closing it properly (one can use try-with-resource).
The #close() on JsonGenerator must only close the resources it created (buffer and others).
In other words, there is something to fix, but I would not revert the change, I would remove the close on the underlying stream at all. And make it clear in the javadoc that it's under the caller responsability to monitor and enforce the lifecycle of the resources it opens.
@jeanouii does not look consistent with the api, JsonGenerator is designed to be autocloseable and to wrap an underlying stream, if it does not close the stream it does leak, even tck are written this way (
Indeed it can be rediscussed in a v3 but not in a minor IMHO.