Gau-Yi / google-api-java-client

Automatically exported from code.google.com/p/google-api-java-client
0 stars 0 forks source link

JsonHttpParser.getContent does not close the inputStream #76

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Version of google-api-java-client (e.g. 1.2.1-alpha)?

1.2.1-alpha

Java environment (e.g. Java 6, Android 2.2, App Engine 1.3.7)?

Java 6

Describe the problem.

In ApacheHttpResponse.java, getContent does not close the inputStream. This 
manifests later with the error:

java.lang.IllegalStateException: Invalid use of
SingleClientConnManager: connection still allocated.
Make sure to release the connection before allocating another one.

How would you expect it to be fixed?

Close the inputStream so that the connection is released.

Original issue reported on code.google.com by danielho...@google.com on 17 Dec 2010 at 8:08

GoogleCodeExporter commented 9 years ago
This is a common source of confusion.  You don't want to close the stream when 
getContent() is called because it hasn't been parsed yet.  getContent() is how 
you access the stream.  To close the stream, please call close() on the 
InputStream returned from getContent().  This can be done via 
HttpResponse.ignore() or one of the parseAs methods.

Original comment by yan...@google.com on 17 Dec 2010 at 9:56

GoogleCodeExporter commented 9 years ago
Yes, that makes sense. FWIW, I am using parseAs(object) for my response, so I 
will have to continue investigating the cause of the issue.

Thanks!

Original comment by danielho...@google.com on 17 Dec 2010 at 10:21

GoogleCodeExporter commented 9 years ago
It looks like the JsonHttpParser method parserForResponse may be the culprit:

public static JsonParser parserForResponse(HttpResponse response) throws 
IOException {
    InputStream content = response.getContent();
    try {
      JsonParser parser = Json.JSON_FACTORY.createJsonParser(content);
      parser.nextToken();
      content = null;
      return parser;
    } finally {
      if (content != null) {
        content.close();
      }
    }
  }

The finally block executes, but content is always null if the parsing is 
successful, and therefore not closed. If my analysis is correct (it may not 
be), please let me know if you'd like me to file a different bug.

Thanks,
Dan

Original comment by danielho...@google.com on 17 Dec 2010 at 11:01

GoogleCodeExporter commented 9 years ago
Again, this is intentional.  This just sets up the JsonParser to prepare the 
JSON content to be parsed from stream.  The caller of this method is expected 
to close the stream.

Original comment by yan...@google.com on 18 Dec 2010 at 4:06

GoogleCodeExporter commented 9 years ago
Thanks walking through this with me. From looking at the buzz-json-oauth-sample 
and my own code, I am consuming the content in the exactly the same way. Eg:

HttpRequest request = this.transport.buildGetRequest();
request.url = CustomerLicenseUrl.forCustomer(this, customerId, true);
CustomerLicense license = request.execute().parseAs(CustomerLicense.class);
return license;

However, executing the above more than once results in an error when forcing 
the ApacheHttpTransport (with 
HttpTransport.setLowLevelHttpTransport(ApacheHttpTransport.INSTANCE);). I 
compared the execution path of the above with the Buzz sample, and noticed that 
the Buzz sample uses the JsonCParser, whereas my sample is using the 
JsonHttpParser. Upon further inspection, I also noticed that the JsonCParser in 
its overridden parse method makes a call to Json.parseAndClose, which closes 
the parser object after parsing completes. In contrast, the 
JsonHttpParser.parse calls Json.parse. When I modify JsonHttpParser.parse to 
call Json.parseAndClose, my code works as expected.

While I could rewrite my code to do:

HttpRequest request = this.transport.buildGetRequest();
request.url = CustomerLicenseUrl.forCustomer(this, customerId, true);
HttpResponse response = request.execute();
CustomerLicense license = response.parseAs(CustomerLicense.class);
response.ignore();
return license;

or equivalent, in order to close the response stream after parsing, it seems 
like the intent is to do what you are doing with the Buzz sample. If there is 
different, preferred convention for this case, could you share it with me? Or, 
is what I have discovered a legitimate issue?

Original comment by danielho...@google.com on 20 Dec 2010 at 8:57

GoogleCodeExporter commented 9 years ago
Aha! Thanks for catching that.  You have indeed discovered a legitimate issue.

I'll try to get this fixed ASAP.

Original comment by yan...@google.com on 20 Dec 2010 at 9:09

GoogleCodeExporter commented 9 years ago

Original comment by yan...@google.com on 20 Dec 2010 at 9:20

GoogleCodeExporter commented 9 years ago

Original comment by yan...@google.com on 6 Jan 2011 at 2:53

GoogleCodeExporter commented 9 years ago

Original comment by yan...@google.com on 8 Jan 2011 at 6:42