Opened 23 months ago

Closed 22 months ago

Last modified 20 months ago

#5858 closed defect (fixed)

WebRequest Reader is not closed

Reported by: asmirnov Assignee:
Priority: P2 Milestone:
Module: Libadblockplus-Android Keywords:
Cc: Blocked By:
Blocking: Platform: Android
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29574567

Description (last modified by asmirnov)

Environment

The reader in AndroidWebRequest (https://github.com/adblockplus/libadblockplus-android/blob/master/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AndroidWebRequest.java#L138) is not closed after reading, allowing possible leak of internal reader buffer (up to 8192 bytes). The bug comes from original adblockplusandroid sources https://github.com/adblockplus/adblockplusandroid/blob/master/src/org/adblockplus/android/AndroidWebRequest.java#L92

It can be detected in StrictMode with

StrictMode.setVmPolicy(new StrictMode.VmPolicy.Builder()
      .detectAll()
      .penaltyLog()
      .penaltyDeath()
      .build());

and can be seen in the log:

10-12 14:19:26.899 5984-5999 E/StrictMode: A resource was acquired at attached stack trace but never released. See java.io.Closeable for information on avoiding resource leaks.
                                           java.lang.Throwable: Explicit termination method 'end' not called
                                               at dalvik.system.CloseGuard.open(CloseGuard.java:184)
                                               at java.util.zip.Inflater.<init>(Inflater.java:82)
                                               at java.util.zip.GZIPInputStream.<init>(GZIPInputStream.java:103)
                                               at java.util.zip.GZIPInputStream.<init>(GZIPInputStream.java:88)
                                               at org.adblockplus.libadblockplus.android.AndroidWebRequest.httpGET(AndroidWebRequest.java:137)
                                               at org.adblockplus.libadblockplus.android.AndroidWebRequestResourceWrapper.httpGET(AndroidWebRequestResourceWrapper.java:140)

Actually there is no serious leak as our files are text files and are read from the beginning to the end, but it would be better to fix it anyway. Note input stream is NOT leaked as it's closed in connection.disconnect() so it relates to reader only. Closing reader forces closing input stream (including GZIPInputStream).

How to reproduce

  1. Enable strict mode (see the code above)
  2. Clean app cache in order to force redownload subscriptions using WebRequest
  3. Make sure you can see output in the log

...

Observed behaviour

Reader is not closed (and leaked).

Expected behaviour

Reader is closed, no log output.

Change History (5)

comment:1 Changed 23 months ago by asmirnov

  • Description modified (diff)

comment:2 Changed 23 months ago by asmirnov

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:3 Changed 22 months ago by abpbot

A commit referencing this issue has landed:
Issue 5858 - WebRequest Reader is not closed

comment:4 Changed 22 months ago by asmirnov

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:5 Changed 20 months ago by abpbot

A commit referencing this issue has landed:
Issue 5858 - WebRequest Reader is not closed

Note: See TracTickets for help on using tickets.