Merge "Close process-spawned streams when the process is destroyed." into dalvik-dev
diff --git a/dalvik/src/main/java/dalvik/system/CloseGuard.java b/dalvik/src/main/java/dalvik/system/CloseGuard.java
index fbb4951..8e53020 100644
--- a/dalvik/src/main/java/dalvik/system/CloseGuard.java
+++ b/dalvik/src/main/java/dalvik/system/CloseGuard.java
@@ -20,7 +20,7 @@
import java.util.logging.Logger;
/**
- * CloseGuard is a mechanism for flagging implict finalizer cleanup of
+ * CloseGuard is a mechanism for flagging implicit finalizer cleanup of
* resources that should have been cleaned up by explicit close
* methods (aka "explicit termination methods" in Effective Java).
* <p>
@@ -54,7 +54,7 @@
* }
* }</pre>
*
- * In usage where the resource to be explictly cleaned up are
+ * In usage where the resource to be explicitly cleaned up are
* allocated after object construction, CloseGuard can protection can
* be deferred. For example: <pre> {@code
* class Bar {
@@ -116,7 +116,7 @@
* Returns a CloseGuard instance. If CloseGuard is enabled, {@code
* #open(String)} can be used to set up the instance to warn on
* failure to close. If CloseGuard is disabled, a non-null no-op
- * instanace is returned.
+ * instance is returned.
*/
public static CloseGuard get() {
if (!enabled()) {
@@ -134,10 +134,10 @@
/**
* If CloseGuard is enabled, {@code open} initializes the instance
- * with a warning that the caller should have explictly called the
+ * with a warning that the caller should have explicitly called the
* {@code closer} method instead of relying on finalization.
*
- * @param closer non-null name of explict termination method
+ * @param closer non-null name of explicit termination method
* @throws NullPointerException if closer is null, regardless of
* whether or not CloseGuard is enabled
*/
@@ -179,6 +179,7 @@
("A resource was acquired at attached stack trace but never released. "
+ "See java.io.Closeable for information on avoiding resource leaks.");
- Logger.global.log(Level.WARNING, message, allocationSite);
+ Logger.getLogger(CloseGuard.class.getName())
+ .log(Level.WARNING, message, allocationSite);
}
}
diff --git a/luni/src/main/java/java/lang/Process.java b/luni/src/main/java/java/lang/Process.java
index 5081f0f..aa6dff5 100644
--- a/luni/src/main/java/java/lang/Process.java
+++ b/luni/src/main/java/java/lang/Process.java
@@ -22,10 +22,38 @@
/**
* Represents an external process. Enables writing to, reading from, destroying,
- * and waiting for the external process, as well as querying its exit value.
+ * and waiting for the external process, as well as querying its exit value. Use
+ * {@link ProcessBuilder} to create processes.
*
- * @see Runtime#exec
- * @see ProcessBuilder#start()
+ * <p>The target process writes its output to two streams, {@code out} and
+ * {@code err}. These streams should be read by this process using {@link
+ * #getInputStream()} and {@link #getErrorStream()} respectively. If these
+ * streams are not read, the target process may block while it awaits buffer
+ * space. If you are not interested in differentiating the out and err
+ * streams, use {@link ProcessBuilder#redirectErrorStream(boolean)
+ * redirectErrorStream(true)} to merge the two streams. This simplifies your
+ * reading code and makes it easier to avoid blocking the target process.
+ *
+ * <p>Running processes hold resources. When a process is no longer used, the
+ * process should be closed by calling {@link #destroy}. This will kill the
+ * process and release the resources that it holds.
+ *
+ * <p>For example, to run {@code /system/bin/ping} to ping {@code android.com}:
+ * <pre> {@code
+ * Process process = new ProcessBuilder()
+ * .command("/system/bin/ping", "android.com")
+ * .redirectErrorStream(true)
+ * .start();
+ * try {
+ * InputStream in = process.getInputStream();
+ * OutputStream out = process.getOutputStream();
+ *
+ * readStream(in);
+ *
+ * } finally {
+ * process.destroy();
+ * }
+ * }</pre>
*/
public abstract class Process {
diff --git a/luni/src/main/java/java/lang/ProcessBuilder.java b/luni/src/main/java/java/lang/ProcessBuilder.java
index 504b79c..5aadade 100644
--- a/luni/src/main/java/java/lang/ProcessBuilder.java
+++ b/luni/src/main/java/java/lang/ProcessBuilder.java
@@ -24,18 +24,14 @@
import java.util.Map;
/**
- * Creates operating system processes.
- *
- * @since 1.5
+ * Creates operating system processes. See {@link Process} for documentation and
+ * example usage.
*/
public final class ProcessBuilder {
private List<String> command;
-
private File directory;
-
private Map<String, String> environment;
-
private boolean redirectErrorStream;
/**
diff --git a/luni/src/main/java/java/lang/ProcessManager.java b/luni/src/main/java/java/lang/ProcessManager.java
index f54640a..d94d051 100644
--- a/luni/src/main/java/java/lang/ProcessManager.java
+++ b/luni/src/main/java/java/lang/ProcessManager.java
@@ -276,6 +276,9 @@
Logger.getLogger(Runtime.class.getName()).log(Level.FINE,
"Failed to destroy process " + id + ".", e);
}
+ IoUtils.closeQuietly(inputStream);
+ IoUtils.closeQuietly(errorStream);
+ IoUtils.closeQuietly(outputStream);
}
public int exitValue() {
diff --git a/luni/src/test/java/libcore/java/lang/ProcessBuilderTest.java b/luni/src/test/java/libcore/java/lang/ProcessBuilderTest.java
index 320cafb..405de6e 100644
--- a/luni/src/test/java/libcore/java/lang/ProcessBuilderTest.java
+++ b/luni/src/test/java/libcore/java/lang/ProcessBuilderTest.java
@@ -17,11 +17,16 @@
package libcore.java.lang;
import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
import java.util.HashMap;
import java.util.Map;
+import libcore.dalvik.system.CloseGuardTester;
import static tests.support.Support_Exec.execAndCheckOutput;
public class ProcessBuilderTest extends junit.framework.TestCase {
+
private static String shell() {
String deviceSh = "/system/bin/sh";
String desktopSh = "/bin/sh";
@@ -49,6 +54,46 @@
execAndCheckOutput(pb, "android\n", "");
}
+ public void testDestroyClosesEverything() throws IOException {
+ Process process = new ProcessBuilder(shell(), "-c", "echo out; echo err 1>&2").start();
+ InputStream in = process.getInputStream();
+ InputStream err = process.getErrorStream();
+ OutputStream out = process.getOutputStream();
+ process.destroy();
+
+ try {
+ in.read();
+ fail();
+ } catch (IOException expected) {
+ }
+ try {
+ err.read();
+ fail();
+ } catch (IOException expected) {
+ }
+ try {
+ /*
+ * We test write+flush because the RI returns a wrapped stream, but
+ * only bothers to close the underlying stream.
+ */
+ out.write(1);
+ out.flush();
+ fail();
+ } catch (IOException expected) {
+ }
+ }
+
+ 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();
+ }
+ }
+
public void testEnvironmentMapForbidsNulls() throws Exception {
ProcessBuilder pb = new ProcessBuilder(shell(), "-c", "echo $A");
Map<String, String> environment = pb.environment();
diff --git a/support/src/test/java/libcore/dalvik/system/CloseGuardTester.java b/support/src/test/java/libcore/dalvik/system/CloseGuardTester.java
new file mode 100644
index 0000000..df2c241
--- /dev/null
+++ b/support/src/test/java/libcore/dalvik/system/CloseGuardTester.java
@@ -0,0 +1,69 @@
+/*
+ * 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.ArrayList;
+import java.util.List;
+import java.util.logging.ConsoleHandler;
+import java.util.logging.Handler;
+import java.util.logging.LogRecord;
+import java.util.logging.Logger;
+import junit.framework.Assert;
+
+public final class CloseGuardTester implements Closeable {
+
+ private final List<LogRecord> logRecords = new ArrayList<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.
+ */
+ System.gc();
+ System.runFinalization();
+ logger.addHandler(logWatcher);
+ }
+
+ public void assertEverythingWasClosed() {
+ System.gc();
+ System.runFinalization();
+
+ 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);
+ }
+}