Skip to content
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

What is the contract for of JsonGenerator#close() on error? #156

Closed
jjspiegel opened this issue Jan 29, 2019 · 4 comments · Fixed by #314
Closed

What is the contract for of JsonGenerator#close() on error? #156

jjspiegel opened this issue Jan 29, 2019 · 4 comments · Fixed by #314
Labels

Comments

@jjspiegel
Copy link

JsonGenerator.close() is defined to close the underlying output source.
https://javaee.github.io/javaee-spec/javadocs/javax/json/stream/JsonGenerator.html#close--

And it is also defined to raise an exception if an incomplete value is generated:
JsonGenerationException - if an incomplete JSON is generated

So, if while writing to a generator, an exception is raised (either from a generation method or from some other cause) calling close() on the partially written generator is very likely to raise another exception.

The question is what is the contract for the underlying output source in this case? Consider the following examples.

In the reference implementation, the following example will not close System.out:

  try (JsonGenerator gen = Json.createGenerator(System.out)) {
    gen.writeStartObject();
    gen.write("foo", "bar");
    // no end object
  } catch (JsonGenerationException e) {
    System.out.println("Gen error: " + e.getMessage());
  } 

However, the following example will close System.out:

  try (JsonGenerator gen = Json.createGenerator(System.out)) {
    gen.writeStartObject();
    gen.write("foo", "bar");
    gen.writeEnd();
  } catch (JsonGenerationException e) {
    System.out.println("Gen error: " + e.getMessage()); // no error in this example
  } 

Is this the intended behavior?

More realistically, a generator may be receiving events from an external source when an error occurs. Given this behavior, it seems useless and potentially troublesome (due to the assured double-error) to close the generator in this case.

@m0mus
Copy link
Contributor

m0mus commented Feb 20, 2019

I guess Javadoc is not clear enough. The underlying stream is closed only if complete JSON object is written. In case of incomplete object JsonGenerationException is thrown and underlying stream is not closed.

jbescos added a commit to jbescos/jsonp that referenced this issue Oct 4, 2021
…#156

Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
@jbescos
Copy link
Member

jbescos commented Oct 6, 2021

  • TCK

lukasj pushed a commit that referenced this issue Oct 13, 2021
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
@lukasj
Copy link
Contributor

lukasj commented Oct 13, 2021

done

@rmannibucau
Copy link

Hi,

seems this introduces a lot of bugs cause JsonGenerator design is to close in all cases the underlying stream (otherwise you can never use the generator as an autocloseable which is the opposite of what we wanted/want).
It is also a very big regression from previous versions and creates leaks.

Therefore this must be fiexd in 2.1.next asap to enforce to always close the underlying stream by spec as before.

Note: it is more than fine to swallow the close exception is any and use addSuppressed if it helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants