search

kirankunaparaju / google-api-java-client

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

Possible Memory Leak? #402

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Version of google-api-java-client 1.6?
Java environment Java 6, Android 2.3

From Calendar API generated code in Calendar.java:
   public com.google.api.services.calendar.model.FreeBusyResponse execute() throws IOException {
    HttpResponse response = executeUnparsed();

    com.google.api.services.calendar.model.FreeBusyResponse result = response.parseAs(
        com.google.api.services.calendar.model.FreeBusyResponse.class);
    result.setResponseHeaders(response.getHeaders());
    return result;
  }

We open an HttpResponse object but we never disconnect it.

Original issue reported on code.google.com by aalb...@google.com on 3 Feb 2012 at 7:32

GoogleCodeExporter commented 9 years ago

Original comment by rmis...@google.com on 3 Feb 2012 at 7:34

GoogleCodeExporter commented 9 years ago 
Yikes

Calling response.disconnect() throws a NPE because in HttpResponse.java:

  public InputStream getContent() throws IOException {
    LowLevelHttpResponse response = this.response;
    if (response == null) {
      return content;
    }
    InputStream content = this.response.getContent();
    this.response = null;

We set the internal response to null.

Original comment by aalb...@google.com on 3 Feb 2012 at 8:06

GoogleCodeExporter commented 9 years ago 
calling disconnect() is not enough. There is also the content that needs to be 
closed.

I think that disconnect() should be renamed to close() and should look like 
this:

    private void close() {
        try {
          final InputStream content = response.getContent();
          if (content != null) {
              content.close();
          }
          response.disconnect();
        } catch (IOException e) {
          // ignore
        }
    }

Calling both disconnect and closing the content fixed my memory leak. (after 
fixing the NPE of course.

Original comment by aalb...@google.com on 3 Feb 2012 at 8:45

GoogleCodeExporter commented 9 years ago 
+1 on calling disconnect.  I highly recommend reading the JavaDoc from the 
Android SDK on this topic:

http://developer.android.com/reference/java/net/HttpURLConnection.html

However, please also read the section on Performance.  We definitely ideally 
want to reuse Sockets, so perhaps we should set the "http.keepAlive" system 
property to "false".  So this should all be investigated carefully.

We might also want to investigate what the correct behavior should be for 
ApacheHttpTransport and UrlFetchHttpTransport.

As far as closing the streams properly, I *think* this has already been 
resolved.  Let me know if you don't think that's the case.

Original comment by yan...@google.com on 16 Feb 2012 at 2:49

GoogleCodeExporter commented 9 years ago

Original comment by yan...@google.com on 16 Feb 2012 at 2:50

GoogleCodeExporter commented 9 years ago

Original comment by rmis...@google.com on 9 Mar 2012 at 5:17

GoogleCodeExporter commented 9 years ago 
Sadly this didn't make the 1.8 release.  But we should really make sure to 
prioritize this for 1.9.  I moved the core of this issue to the HTTP project:

http://code.google.com/p/google-http-java-client/issues/detail?id=78

but the change will likely have an impact on this project as well.

Original comment by yan...@google.com on 27 Mar 2012 at 2:48

GoogleCodeExporter commented 9 years ago

Original comment by rmis...@google.com on 14 May 2012 at 2:21

GoogleCodeExporter commented 9 years ago

Original comment by yan...@google.com on 31 May 2012 at 12:47

GoogleCodeExporter commented 9 years ago 
http://codereview.appspot.com/6255069/

Original comment by rmis...@google.com on 6 Jun 2012 at 4:40