diff options
8 files changed, 365 insertions, 127 deletions
diff --git a/dalvik/src/test/java/dalvik/system/CloseGuardMonitor.java b/dalvik/src/test/java/dalvik/system/CloseGuardMonitor.java new file mode 100644 index 0000000..b5bf380 --- /dev/null +++ b/dalvik/src/test/java/dalvik/system/CloseGuardMonitor.java @@ -0,0 +1,119 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package dalvik.system; + +import dalvik.system.CloseGuard.Reporter; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.lang.ref.WeakReference; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; + +/** + * Provides support for detecting issues found by {@link CloseGuard} from within tests. + * + * <p>This is a best effort as it relies on both {@link CloseGuard} being enabled and being able to + * force a GC and finalization, none of which are directly controllable by this. + * + * <p>This is loaded using reflection by the AbstractResourceLeakageDetectorTestCase class as that + * class needs to run on the reference implementation which does not have this class. It implements + * {@link Runnable} because that is simpler than trying to manage a specialized interface. + * + * @hide + */ +public class CloseGuardMonitor implements Runnable { + /** + * The {@link Reporter} instance used to receive warnings from {@link CloseGuard}. + */ + private final Reporter closeGuardReporter; + + /** + * The list of allocation sites that {@link CloseGuard} has reported as not being released. + * + * <p>Is thread safe as this will be called during finalization and so there are no guarantees + * as to whether it will be called concurrently or not. + */ + private final List<Throwable> closeGuardAllocationSites = new CopyOnWriteArrayList<>(); + + /** + * Default constructor required for reflection. + */ + public CloseGuardMonitor() { + System.logI("Creating CloseGuard monitor"); + + // Save current reporter. + closeGuardReporter = CloseGuard.getReporter(); + + // Override the reporter with our own which collates the allocation sites. + CloseGuard.setReporter(new Reporter() { + @Override + public void report(String message, Throwable allocationSite) { + // Ignore message as it's always the same. + closeGuardAllocationSites.add(allocationSite); + } + }); + } + + /** + * Check to see whether any resources monitored by {@link CloseGuard} were not released before + * they were garbage collected. + */ + @Override + public void run() { + // Create a weak reference to an object so that we can detect when it is garbage collected. + WeakReference<Object> reference = new WeakReference<>(new Object()); + + try { + // 'Force' a GC and finalize to cause CloseGuards to report warnings. Doesn't loop + // forever as there are no guarantees that the following code does anything at all so + // don't want a potential infinite loop. + Runtime runtime = Runtime.getRuntime(); + for (int i = 0; i < 20; ++i) { + runtime.gc(); + System.runFinalization(); + try { + Thread.sleep(1); + } catch (InterruptedException e) { + throw new AssertionError(e); + } + + // Check to see if the weak reference has been garbage collected. + if (reference.get() == null) { + System.logI("Sentry object has been freed so assuming CloseGuards have reported" + + " any resource leakages"); + break; + } + } + } finally { + // Restore the reporter. + CloseGuard.setReporter(closeGuardReporter); + } + + if (!closeGuardAllocationSites.isEmpty()) { + StringWriter writer = new StringWriter(); + PrintWriter printWriter = new PrintWriter(writer); + int i = 0; + for (Throwable allocationSite : closeGuardAllocationSites) { + printWriter.print(++i); + printWriter.print(") "); + allocationSite.printStackTrace(printWriter); + printWriter.println(" --------------------------------"); + } + throw new AssertionError("Potential resource leakage detected:\n" + writer); + } + } +} diff --git a/luni/src/main/java/java/util/zip/ZipFile.java b/luni/src/main/java/java/util/zip/ZipFile.java index 4b3e431..43e8567 100644 --- a/luni/src/main/java/java/util/zip/ZipFile.java +++ b/luni/src/main/java/java/util/zip/ZipFile.java @@ -33,6 +33,7 @@ import java.util.Iterator; import java.util.LinkedHashMap; import libcore.io.BufferIterator; import libcore.io.HeapBufferIterator; +import libcore.io.IoUtils; import libcore.io.Streams; /** @@ -199,7 +200,19 @@ public class ZipFile implements Closeable, ZipConstants { raf = new RandomAccessFile(filename, "r"); - readCentralDir(); + // Make sure to close the RandomAccessFile if reading the central directory fails. + boolean mustCloseFile = true; + try { + readCentralDir(); + + // Read succeeded so do not close the underlying RandomAccessFile. + mustCloseFile = false; + } finally { + if (mustCloseFile) { + IoUtils.closeQuietly(raf); + } + } + guard.open("close"); } diff --git a/luni/src/test/java/libcore/java/lang/ProcessBuilderTest.java b/luni/src/test/java/libcore/java/lang/ProcessBuilderTest.java index d03ae65..9766cef 100644 --- a/luni/src/test/java/libcore/java/lang/ProcessBuilderTest.java +++ b/luni/src/test/java/libcore/java/lang/ProcessBuilderTest.java @@ -22,10 +22,10 @@ import java.io.InputStream; import java.io.OutputStream; import java.util.HashMap; import java.util.Map; -import libcore.dalvik.system.CloseGuardTester; +import libcore.java.util.AbstractResourceLeakageDetectorTestCase; import static tests.support.Support_Exec.execAndCheckOutput; -public class ProcessBuilderTest extends junit.framework.TestCase { +public class ProcessBuilderTest extends AbstractResourceLeakageDetectorTestCase { private static String shell() { String deviceSh = "/system/bin/sh"; @@ -84,14 +84,8 @@ public class ProcessBuilderTest extends junit.framework.TestCase { } public void testDestroyDoesNotLeak() throws IOException { - CloseGuardTester closeGuardTester = new CloseGuardTester(); - try { - Process process = new ProcessBuilder(shell(), "-c", "echo out; echo err 1>&2").start(); - process.destroy(); - closeGuardTester.assertEverythingWasClosed(); - } finally { - closeGuardTester.close(); - } + Process process = new ProcessBuilder(shell(), "-c", "echo out; echo err 1>&2").start(); + process.destroy(); } public void testEnvironmentMapForbidsNulls() throws Exception { diff --git a/luni/src/test/java/libcore/java/net/URLConnectionTest.java b/luni/src/test/java/libcore/java/net/URLConnectionTest.java index b40976e..e945e60 100644 --- a/luni/src/test/java/libcore/java/net/URLConnectionTest.java +++ b/luni/src/test/java/libcore/java/net/URLConnectionTest.java @@ -21,9 +21,7 @@ import com.google.mockwebserver.MockResponse; import com.google.mockwebserver.MockWebServer; import com.google.mockwebserver.RecordedRequest; import com.google.mockwebserver.SocketPolicy; -import dalvik.system.CloseGuard; import java.io.ByteArrayOutputStream; -import java.io.Closeable; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -31,7 +29,6 @@ import java.io.OutputStream; import java.net.Authenticator; import java.net.CacheRequest; import java.net.CacheResponse; -import java.net.ConnectException; import java.net.HttpRetryException; import java.net.HttpURLConnection; import java.net.InetAddress; @@ -68,9 +65,8 @@ import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.TrustManager; import javax.net.ssl.X509TrustManager; -import junit.framework.TestCase; -import libcore.java.lang.ref.FinalizationTester; import libcore.java.security.TestKeyStore; +import libcore.java.util.AbstractResourceLeakageDetectorTestCase; import libcore.javax.net.ssl.TestSSLContext; import tests.net.StuckServer; @@ -80,7 +76,8 @@ import static com.google.mockwebserver.SocketPolicy.FAIL_HANDSHAKE; import static com.google.mockwebserver.SocketPolicy.SHUTDOWN_INPUT_AT_END; import static com.google.mockwebserver.SocketPolicy.SHUTDOWN_OUTPUT_AT_END; -public final class URLConnectionTest extends TestCase { +public final class URLConnectionTest extends AbstractResourceLeakageDetectorTestCase { + private MockWebServer server; private HttpResponseCache cache; private String hostName; @@ -101,8 +98,10 @@ public final class URLConnectionTest extends TestCase { System.clearProperty("https.proxyHost"); System.clearProperty("https.proxyPort"); server.shutdown(); + server = null; if (cache != null) { cache.delete(); + cache = null; } super.tearDown(); } @@ -797,47 +796,17 @@ public final class URLConnectionTest extends TestCase { } public void testDisconnectAfterOnlyResponseCodeCausesNoCloseGuardWarning() throws IOException { - CloseGuardGuard guard = new CloseGuardGuard(); - try { - server.enqueue(new MockResponse() - .setBody(gzip("ABCABCABC".getBytes("UTF-8"))) - .addHeader("Content-Encoding: gzip")); - server.play(); + server.enqueue(new MockResponse() + .setBody(gzip("ABCABCABC".getBytes("UTF-8"))) + .addHeader("Content-Encoding: gzip")); + server.play(); - HttpURLConnection connection = (HttpURLConnection) server.getUrl("/").openConnection(); + HttpURLConnection connection = (HttpURLConnection) server.getUrl("/").openConnection(); + try { assertEquals(200, connection.getResponseCode()); - connection.disconnect(); - connection = null; - assertFalse(guard.wasCloseGuardCalled()); } finally { - guard.close(); - } - } - - public static class CloseGuardGuard implements Closeable, CloseGuard.Reporter { - private final CloseGuard.Reporter oldReporter = CloseGuard.getReporter(); - - private AtomicBoolean closeGuardCalled = new AtomicBoolean(); - - public CloseGuardGuard() { - CloseGuard.setReporter(this); - } - - @Override public void report(String message, Throwable allocationSite) { - oldReporter.report(message, allocationSite); - closeGuardCalled.set(true); - } - - public boolean wasCloseGuardCalled() { - FinalizationTester.induceFinalization(); - close(); - return closeGuardCalled.get(); - } - - @Override public void close() { - CloseGuard.setReporter(oldReporter); + connection.disconnect(); } - } public void testDefaultRequestProperty() throws Exception { @@ -2272,11 +2241,15 @@ public final class URLConnectionTest extends TestCase { HttpsURLConnection connection = (HttpsURLConnection) server.getUrl("/").openConnection(); connection.setSSLSocketFactory(testSSLContext.clientContext.getSocketFactory()); connection.connect(); - assertNotNull(connection.getHostnameVerifier()); - assertNull(connection.getLocalCertificates()); - assertNotNull(connection.getServerCertificates()); - assertNotNull(connection.getCipherSuite()); - assertNotNull(connection.getPeerPrincipal()); + try { + assertNotNull(connection.getHostnameVerifier()); + assertNull(connection.getLocalCertificates()); + assertNotNull(connection.getServerCertificates()); + assertNotNull(connection.getCipherSuite()); + assertNotNull(connection.getPeerPrincipal()); + } finally { + connection.disconnect(); + } } /** diff --git a/luni/src/test/java/libcore/java/util/AbstractResourceLeakageDetectorTestCase.java b/luni/src/test/java/libcore/java/util/AbstractResourceLeakageDetectorTestCase.java new file mode 100644 index 0000000..5ea67d3 --- /dev/null +++ b/luni/src/test/java/libcore/java/util/AbstractResourceLeakageDetectorTestCase.java @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package libcore.java.util; + +import junit.framework.TestCase; + +/** + * Ensures that resources used within a test are cleaned up; will detect problems with tests and + * also with runtime. + */ +public abstract class AbstractResourceLeakageDetectorTestCase extends TestCase { + /** + * The leakage detector. + */ + private ResourceLeakageDetector detector; + + @Override + protected void setUp() throws Exception { + detector = ResourceLeakageDetector.newDetector(); + } + + @Override + protected void tearDown() throws Exception { + // If available check for resource leakage. At this point it is impossible to determine + // whether the test has thrown an exception. If it has then the exception thrown by this + // could hide that test failure; it largely depends on the test runner. + if (detector != null) { + detector.checkForLeaks(); + } + } +} diff --git a/luni/src/test/java/libcore/java/util/ResourceLeakageDetector.java b/luni/src/test/java/libcore/java/util/ResourceLeakageDetector.java new file mode 100644 index 0000000..954665a --- /dev/null +++ b/luni/src/test/java/libcore/java/util/ResourceLeakageDetector.java @@ -0,0 +1,90 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package libcore.java.util; + +/** + * Detects resource leakages for resources that are protected by <code>CloseGuard</code> mechanism. + * + * <p>If multiple instances of this are active at the same time, i.e. have been created but not yet + * had their {@link #checkForLeaks()} method called then while they will report all the leakages + * detected they may report the leakages caused by the code being tested by another detector. + * + * <p>The underlying CloseGuardMonitor is loaded using reflection to ensure that this will run, + * albeit doing nothing, on the reference implementation. + */ +public class ResourceLeakageDetector { + /** The class for the CloseGuardMonitor, null if not supported. */ + private static final Class<?> CLOSE_GUARD_MONITOR_CLASS; + + static { + ClassLoader classLoader = ResourceLeakageDetector.class.getClassLoader(); + Class<?> clazz; + try { + // Make sure that the CloseGuard class exists; this ensures that this is not running + // on a RI JVM. + classLoader.loadClass("dalvik.system.CloseGuard"); + + // Load the monitor class for later instantiation. + clazz = classLoader.loadClass("dalvik.system.CloseGuardMonitor"); + + } catch (ClassNotFoundException e) { + System.err.println("Resource leakage will not be detected; " + + "this is expected in the reference implementation"); + e.printStackTrace(System.err); + + // Ignore, probably running in reference implementation. + clazz = null; + } + + CLOSE_GUARD_MONITOR_CLASS = clazz; + } + + /** + * The underlying CloseGuardMonitor that will perform the post test checks for resource + * leakage. + */ + private Runnable postTestChecker; + + /** + * Create a new detector. + * + * @return The new {@link ResourceLeakageDetector}, its {@link #checkForLeaks()} method must be + * called otherwise it will not clean up properly after itself. + */ + public static ResourceLeakageDetector newDetector() + throws Exception { + return new ResourceLeakageDetector(); + } + + private ResourceLeakageDetector() + throws Exception { + if (CLOSE_GUARD_MONITOR_CLASS != null) { + postTestChecker = (Runnable) CLOSE_GUARD_MONITOR_CLASS.newInstance(); + } + } + + /** + * Detect any leaks that have arisen since this was created. + * + * @throws Exception If any leaks were detected. + */ + public void checkForLeaks() throws Exception { + // If available check for resource leakage. + if (postTestChecker != null) { + postTestChecker.run(); + } + } +} diff --git a/luni/src/test/java/libcore/java/util/ResourceLeakageDetectorTest.java b/luni/src/test/java/libcore/java/util/ResourceLeakageDetectorTest.java new file mode 100644 index 0000000..d86c9f2 --- /dev/null +++ b/luni/src/test/java/libcore/java/util/ResourceLeakageDetectorTest.java @@ -0,0 +1,73 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package libcore.java.util; + +import dalvik.system.CloseGuard; +import junit.framework.TestCase; + +/** + * Test for {@link ResourceLeakageDetector} + */ +public class ResourceLeakageDetectorTest extends TestCase { + /** + * This test will not work on RI as it does not support the <code>CloseGuard</code> or similar + * mechanism. + */ + public void testDetectsUnclosedCloseGuard() throws Exception { + ResourceLeakageDetector detector = ResourceLeakageDetector.newDetector(); + try { + CloseGuard closeGuard = createCloseGuard(); + closeGuard.open("open"); + } finally { + try { + System.logI("Checking for leaks"); + detector.checkForLeaks(); + fail(); + } catch (AssertionError expected) { + } + } + } + + public void testIgnoresClosedCloseGuard() throws Exception { + ResourceLeakageDetector detector = ResourceLeakageDetector.newDetector(); + try { + CloseGuard closeGuard = createCloseGuard(); + closeGuard.open("open"); + closeGuard.close(); + } finally { + detector.checkForLeaks(); + } + } + + /** + * Private method to ensure that the CloseGuard object is garbage collected. + */ + private CloseGuard createCloseGuard() { + final CloseGuard closeGuard = CloseGuard.get(); + new Object() { + @Override + protected void finalize() throws Throwable { + try { + closeGuard.warnIfOpen(); + } finally { + super.finalize(); + } + } + }; + + return closeGuard; + } +} diff --git a/support/src/test/java/libcore/dalvik/system/CloseGuardTester.java b/support/src/test/java/libcore/dalvik/system/CloseGuardTester.java deleted file mode 100644 index e82d33d..0000000 --- a/support/src/test/java/libcore/dalvik/system/CloseGuardTester.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright (C) 2010 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package libcore.dalvik.system; - -import dalvik.system.CloseGuard; -import java.io.Closeable; -import java.io.IOException; -import java.util.List; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.logging.ConsoleHandler; -import java.util.logging.Handler; -import java.util.logging.LogRecord; -import java.util.logging.Logger; -import junit.framework.Assert; -import libcore.java.lang.ref.FinalizationTester; - -public final class CloseGuardTester implements Closeable { - - private final List<LogRecord> logRecords = new CopyOnWriteArrayList<LogRecord>(); - private final Logger logger = Logger.getLogger(CloseGuard.class.getName()); - - private final Handler logWatcher = new Handler() { - @Override public void close() {} - @Override public void flush() {} - @Override public void publish(LogRecord record) { - logRecords.add(record); - } - }; - - public CloseGuardTester() { - /* - * Collect immediately before we start monitoring the CloseGuard logs. - * This lowers the chance that we'll report an unrelated leak. - */ - FinalizationTester.induceFinalization(); - logger.addHandler(logWatcher); - } - - public void assertEverythingWasClosed() { - FinalizationTester.induceFinalization(); - - if (!logRecords.isEmpty()) { - // print the log records with the output of this test - for (LogRecord leak : logRecords) { - new ConsoleHandler().publish(leak); - } - Assert.fail("CloseGuard detected unclosed resources!"); - } - } - - @Override public void close() throws IOException { - Logger.getLogger(CloseGuard.class.getName()).removeHandler(logWatcher); - } -} |