AboutHomeContent.getRecommendedAddonsStream() resource leak because ZipFile was not closed

RESOLVED FIXED

Status

()

defect
P4
normal
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: cpeterson, Assigned: blassey)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment)

Reporter

Description

8 years ago
Using a Nightly or local build with Android StrictMode enabled, logcat reports that AboutHomeContent.getRecommendedAddonsStream() does not call ZipFile.close().

I can repro these warnings on ICS, but not Gingerbread.

E/StrictMode( 2332): A resource was acquired at attached stack trace but never released. See java.io.Closeable for information on avoiding resource leaks.
E/StrictMode( 2332): java.lang.Throwable: Explicit termination method 'close' not called
E/StrictMode( 2332): 	at dalvik.system.CloseGuard.open(CloseGuard.java:184)
E/StrictMode( 2332): 	at java.util.zip.ZipFile.<init>(ZipFile.java:133)
E/StrictMode( 2332): 	at java.util.zip.ZipFile.<init>(ZipFile.java:103)
E/StrictMode( 2332): 	at org.mozilla.gecko.AboutHomeContent.getRecommendedAddonsStream(AboutHomeContent.java:174)
E/StrictMode( 2332): 	at org.mozilla.gecko.AboutHomeContent$4.run(AboutHomeContent.java:184)
E/StrictMode( 2332): 	at android.os.Handler.handleCallback(Handler.java:605)
E/StrictMode( 2332): 	at android.os.Handler.dispatchMessage(Handler.java:92)
E/StrictMode( 2332): 	at android.os.Looper.loop(Looper.java:137)
E/StrictMode( 2332): 	at org.mozilla.gecko.GeckoAppShell$LooperThread.run(GeckoAppShell.java:153)

E/StrictMode( 2332): A resource was acquired at attached stack trace but never released. See java.io.Closeable for information on avoiding resource leaks.
E/StrictMode( 2332): java.lang.Throwable: Explicit termination method 'close' not called
E/StrictMode( 2332): 	at dalvik.system.CloseGuard.open(CloseGuard.java:184)
E/StrictMode( 2332): 	at java.io.RandomAccessFile.<init>(RandomAccessFile.java:128)
E/StrictMode( 2332): 	at java.io.RandomAccessFile.<init>(RandomAccessFile.java:150)
E/StrictMode( 2332): 	at java.util.zip.ZipFile.<init>(ZipFile.java:130)
E/StrictMode( 2332): 	at java.util.zip.ZipFile.<init>(ZipFile.java:103)
E/StrictMode( 2332): 	at org.mozilla.gecko.AboutHomeContent.getRecommendedAddonsStream(AboutHomeContent.java:174)
E/StrictMode( 2332): 	at org.mozilla.gecko.AboutHomeContent$4.run(AboutHomeContent.java:184)
E/StrictMode( 2332): 	at android.os.Handler.handleCallback(Handler.java:605)
E/StrictMode( 2332): 	at android.os.Handler.dispatchMessage(Handler.java:92)
E/StrictMode( 2332): 	at android.os.Looper.loop(Looper.java:137)
E/StrictMode( 2332): 	at org.mozilla.gecko.GeckoAppShell$LooperThread.run(GeckoAppShell.java:153)

E/StrictMode( 2332): A resource was acquired at attached stack trace but never released. See java.io.Closeable for information on avoiding resource leaks.
E/StrictMode( 2332): java.lang.Throwable: Explicit termination method 'end' not called
E/StrictMode( 2332): 	at dalvik.system.CloseGuard.open(CloseGuard.java:184)
E/StrictMode( 2332): 	at java.util.zip.Inflater.<init>(Inflater.java:82)
E/StrictMode( 2332): 	at java.util.zip.ZipFile.getInputStream(ZipFile.java:270)
E/StrictMode( 2332): 	at org.mozilla.gecko.AboutHomeContent.getRecommendedAddonsStream(AboutHomeContent.java:176)
E/StrictMode( 2332): 	at org.mozilla.gecko.AboutHomeContent$4.run(AboutHomeContent.java:184)
E/StrictMode( 2332): 	at android.os.Handler.handleCallback(Handler.java:605)
E/StrictMode( 2332): 	at android.os.Handler.dispatchMessage(Handler.java:92)
E/StrictMode( 2332): 	at android.os.Looper.loop(Looper.java:137)
E/StrictMode( 2332): 	at org.mozilla.gecko.GeckoAppShell$LooperThread.run(GeckoAppShell.java:153)
Reporter

Comment 1

8 years ago
You may need to apply the StrictMode patches from bug 708114 to repro the warnings.
Assignee: nobody → blassey.bugs
Priority: -- → P4
Posted patch patchSplinter Review
Attachment #582439 - Flags: review?(mark.finkle)
Comment on attachment 582439 [details] [diff] [review]
patch

drive by:


+            if (zip == null)
+                return null;

and

+            if (fileEntry == null)
+                return null;

and

+            if (zip != null)

Don't need those if.  Its java, after all.  Just add a catch that returns null.


+        try {
+            zip = new ZipFile(applicationPackage);
+            ZipEntry fileEntry = zip.getEntry("recommended-addons.json");
+            return zip.getInputStream(fileEntry);
+        } finally {
+            if (zip != null)
+                zip.close();
+        } catch (Exception) {
+            return null;
+        }
Attachment #582439 - Flags: review?(mark.finkle) → review+
pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/43fd5d2ede79

> Don't need those if.  Its java, after all.  Just add a catch that returns null.
I don't like throwing unnecessary exceptions
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/43fd5d2ede79
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Reporter

Comment 6

8 years ago
@blassey:

1. I don't think your code change will work. When your finally block calls zip.close(), the zip's InputStream will also be closed. This method will always return a closed (and useless) InputStream.

http://docs.oracle.com/javase/1.5.0/docs/api/java/util/zip/ZipFile.html#close%28%29

2. Java |new| does not return null. You do not need to check if (zip == null).
You need to log in before you can comment on or make changes to this bug.