Zygote: Improve logging and error handling during connections.

Before this change, we were throwing a checked exception on success
and returning on failure. This made it hard to figure out where / when
something was going wrong. This change switches things around to throw
a RuntimeException when something goes wrong and to return a Runnable
on success. This lets us make stronger assertions in both the parent
and the child process about their state and expected return values.

This change also upgrades the severity of several errors that we would
earlier just ignore. We will now reject the command and terminate the
connection to the client when we encounter these errors such as:

- Malformed arguments
- Any ZygoteSecurityException, such as thown thrown by
  applyUidSecurityPolicy and applyInvokeWithSecurityPolicy.
- Any error in setting up pipes etc. to facilitate communication
  with child processes.

Bug: 13618569
Test: Manual
Change-Id: Id931d44135ae2e4ede1bbac6a4b187f6c139e1fd
diff --git a/core/java/com/android/internal/os/RuntimeInit.java b/core/java/com/android/internal/os/RuntimeInit.java
index f4be128..66475e4 100644
--- a/core/java/com/android/internal/os/RuntimeInit.java
+++ b/core/java/com/android/internal/os/RuntimeInit.java
@@ -31,6 +31,7 @@
 import com.android.internal.logging.AndroidConfig;
 import com.android.server.NetworkManagementSocketTagger;
 import dalvik.system.VMRuntime;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.TimeZone;
@@ -228,8 +229,8 @@
      * @param argv Argument vector for main()
      * @param classLoader the classLoader to load {@className} with
      */
-    private static void invokeStaticMain(String className, String[] argv, ClassLoader classLoader)
-            throws Zygote.MethodAndArgsCaller {
+    private static Runnable findStaticMain(String className, String[] argv,
+            ClassLoader classLoader) {
         Class<?> cl;
 
         try {
@@ -263,7 +264,7 @@
          * clears up all the stack frames that were required in setting
          * up the process.
          */
-        throw new Zygote.MethodAndArgsCaller(m, argv);
+        return new MethodAndArgsCaller(m, argv);
     }
 
     public static final void main(String[] argv) {
@@ -286,8 +287,8 @@
         if (DEBUG) Slog.d(TAG, "Leaving RuntimeInit!");
     }
 
-    protected static void applicationInit(int targetSdkVersion, String[] argv, ClassLoader classLoader)
-            throws Zygote.MethodAndArgsCaller {
+    protected static Runnable applicationInit(int targetSdkVersion, String[] argv,
+            ClassLoader classLoader) {
         // If the application calls System.exit(), terminate the process
         // immediately without running any shutdown hooks.  It is not possible to
         // shutdown an Android application gracefully.  Among other things, the
@@ -300,20 +301,13 @@
         VMRuntime.getRuntime().setTargetHeapUtilization(0.75f);
         VMRuntime.getRuntime().setTargetSdkVersion(targetSdkVersion);
 
-        final Arguments args;
-        try {
-            args = new Arguments(argv);
-        } catch (IllegalArgumentException ex) {
-            Slog.e(TAG, ex.getMessage());
-            // let the process exit
-            return;
-        }
+        final Arguments args = new Arguments(argv);
 
         // The end of of the RuntimeInit event (see #zygoteInit).
         Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER);
 
         // Remaining arguments are passed to the start class's static main
-        invokeStaticMain(args.startClass, args.startArgs, classLoader);
+        return findStaticMain(args.startClass, args.startArgs, classLoader);
     }
 
     /**
@@ -422,4 +416,37 @@
             System.arraycopy(args, curArg, startArgs, 0, startArgs.length);
         }
     }
+
+    /**
+     * Helper class which holds a method and arguments and can call them. This is used as part of
+     * a trampoline to get rid of the initial process setup stack frames.
+     */
+    static class MethodAndArgsCaller implements Runnable {
+        /** method to call */
+        private final Method mMethod;
+
+        /** argument array */
+        private final String[] mArgs;
+
+        public MethodAndArgsCaller(Method method, String[] args) {
+            mMethod = method;
+            mArgs = args;
+        }
+
+        public void run() {
+            try {
+                mMethod.invoke(null, new Object[] { mArgs });
+            } catch (IllegalAccessException ex) {
+                throw new RuntimeException(ex);
+            } catch (InvocationTargetException ex) {
+                Throwable cause = ex.getCause();
+                if (cause instanceof RuntimeException) {
+                    throw (RuntimeException) cause;
+                } else if (cause instanceof Error) {
+                    throw (Error) cause;
+                }
+                throw new RuntimeException(ex);
+            }
+        }
+    }
 }
diff --git a/core/java/com/android/internal/os/WebViewZygoteInit.java b/core/java/com/android/internal/os/WebViewZygoteInit.java
index 58e4a3e..7f46a0c 100644
--- a/core/java/com/android/internal/os/WebViewZygoteInit.java
+++ b/core/java/com/android/internal/os/WebViewZygoteInit.java
@@ -69,8 +69,7 @@
         }
 
         @Override
-        protected boolean handlePreloadPackage(String packagePath, String libsPath,
-                                               String cacheKey) {
+        protected void handlePreloadPackage(String packagePath, String libsPath, String cacheKey) {
             Log.i(TAG, "Beginning package preload");
             // Ask ApplicationLoaders to create and cache a classloader for the WebView APK so that
             // our children will reuse the same classloader instead of creating their own.
@@ -106,12 +105,10 @@
                 DataOutputStream socketOut = getSocketOutputStream();
                 socketOut.writeInt(preloadSucceeded ? 1 : 0);
             } catch (IOException ioe) {
-                Log.e(TAG, "Error writing to command socket", ioe);
-                return true;
+                throw new IllegalStateException("Error writing to command socket", ioe);
             }
 
             Log.i(TAG, "Package preload done");
-            return false;
         }
     }
 
@@ -125,16 +122,23 @@
             throw new RuntimeException("Failed to setpgid(0,0)", ex);
         }
 
+        final Runnable caller;
         try {
             sServer.registerServerSocket("webview_zygote");
-            sServer.runSelectLoop(TextUtils.join(",", Build.SUPPORTED_ABIS));
-            sServer.closeServerSocket();
-        } catch (Zygote.MethodAndArgsCaller caller) {
-            caller.run();
+            // The select loop returns early in the child process after a fork and
+            // loops forever in the zygote.
+            caller = sServer.runSelectLoop(TextUtils.join(",", Build.SUPPORTED_ABIS));
         } catch (RuntimeException e) {
             Log.e(TAG, "Fatal exception:", e);
+            throw e;
+        } finally {
+            sServer.closeServerSocket();
         }
 
-        System.exit(0);
+        // We're in the child process and have exited the select loop. Proceed to execute the
+        // command.
+        if (caller != null) {
+            caller.run();
+        }
     }
 }
diff --git a/core/java/com/android/internal/os/WrapperInit.java b/core/java/com/android/internal/os/WrapperInit.java
index 608bc9f..89328b2 100644
--- a/core/java/com/android/internal/os/WrapperInit.java
+++ b/core/java/com/android/internal/os/WrapperInit.java
@@ -25,7 +25,6 @@
 import android.system.StructCapUserHeader;
 import android.util.BootTimingsTraceLog;
 import android.util.Slog;
-import com.android.internal.os.Zygote.MethodAndArgsCaller;
 import dalvik.system.VMRuntime;
 import java.io.DataOutputStream;
 import java.io.FileDescriptor;
@@ -61,37 +60,35 @@
      * @param args The command-line arguments.
      */
     public static void main(String[] args) {
-        try {
-            // Parse our mandatory arguments.
-            int fdNum = Integer.parseInt(args[0], 10);
-            int targetSdkVersion = Integer.parseInt(args[1], 10);
+        // Parse our mandatory arguments.
+        int fdNum = Integer.parseInt(args[0], 10);
+        int targetSdkVersion = Integer.parseInt(args[1], 10);
 
-            // Tell the Zygote what our actual PID is (since it only knows about the
-            // wrapper that it directly forked).
-            if (fdNum != 0) {
-                try {
-                    FileDescriptor fd = new FileDescriptor();
-                    fd.setInt$(fdNum);
-                    DataOutputStream os = new DataOutputStream(new FileOutputStream(fd));
-                    os.writeInt(Process.myPid());
-                    os.close();
-                    IoUtils.closeQuietly(fd);
-                } catch (IOException ex) {
-                    Slog.d(TAG, "Could not write pid of wrapped process to Zygote pipe.", ex);
-                }
+        // Tell the Zygote what our actual PID is (since it only knows about the
+        // wrapper that it directly forked).
+        if (fdNum != 0) {
+            try {
+                FileDescriptor fd = new FileDescriptor();
+                fd.setInt$(fdNum);
+                DataOutputStream os = new DataOutputStream(new FileOutputStream(fd));
+                os.writeInt(Process.myPid());
+                os.close();
+                IoUtils.closeQuietly(fd);
+            } catch (IOException ex) {
+                Slog.d(TAG, "Could not write pid of wrapped process to Zygote pipe.", ex);
             }
-
-            // Mimic system Zygote preloading.
-            ZygoteInit.preload(new BootTimingsTraceLog("WrapperInitTiming",
-                    Trace.TRACE_TAG_DALVIK));
-
-            // Launch the application.
-            String[] runtimeArgs = new String[args.length - 2];
-            System.arraycopy(args, 2, runtimeArgs, 0, runtimeArgs.length);
-            WrapperInit.wrapperInit(targetSdkVersion, runtimeArgs);
-        } catch (Zygote.MethodAndArgsCaller caller) {
-            caller.run();
         }
+
+        // Mimic system Zygote preloading.
+        ZygoteInit.preload(new BootTimingsTraceLog("WrapperInitTiming",
+                Trace.TRACE_TAG_DALVIK));
+
+        // Launch the application.
+        String[] runtimeArgs = new String[args.length - 2];
+        System.arraycopy(args, 2, runtimeArgs, 0, runtimeArgs.length);
+        Runnable r = wrapperInit(targetSdkVersion, runtimeArgs);
+
+        r.run();
     }
 
     /**
@@ -142,8 +139,7 @@
      * @param targetSdkVersion target SDK version
      * @param argv arg strings
      */
-    private static void wrapperInit(int targetSdkVersion, String[] argv)
-            throws Zygote.MethodAndArgsCaller {
+    private static Runnable wrapperInit(int targetSdkVersion, String[] argv) {
         if (RuntimeInit.DEBUG) {
             Slog.d(RuntimeInit.TAG, "RuntimeInit: Starting application from wrapper");
         }
@@ -165,7 +161,7 @@
 
         // Perform the same initialization that would happen after the Zygote forks.
         Zygote.nativePreApplicationInit();
-        RuntimeInit.applicationInit(targetSdkVersion, argv, classLoader);
+        return RuntimeInit.applicationInit(targetSdkVersion, argv, classLoader);
     }
 
     /**
diff --git a/core/java/com/android/internal/os/Zygote.java b/core/java/com/android/internal/os/Zygote.java
index 0106b81..a9350db 100644
--- a/core/java/com/android/internal/os/Zygote.java
+++ b/core/java/com/android/internal/os/Zygote.java
@@ -221,39 +221,4 @@
             command.append(" '").append(arg.replace("'", "'\\''")).append("'");
         }
     }
-
-    /**
-     * Helper exception class which holds a method and arguments and
-     * can call them. This is used as part of a trampoline to get rid of
-     * the initial process setup stack frames.
-     */
-    public static class MethodAndArgsCaller extends Exception
-            implements Runnable {
-        /** method to call */
-        private final Method mMethod;
-
-        /** argument array */
-        private final String[] mArgs;
-
-        public MethodAndArgsCaller(Method method, String[] args) {
-            mMethod = method;
-            mArgs = args;
-        }
-
-        public void run() {
-            try {
-                mMethod.invoke(null, new Object[] { mArgs });
-            } catch (IllegalAccessException ex) {
-                throw new RuntimeException(ex);
-            } catch (InvocationTargetException ex) {
-                Throwable cause = ex.getCause();
-                if (cause instanceof RuntimeException) {
-                    throw (RuntimeException) cause;
-                } else if (cause instanceof Error) {
-                    throw (Error) cause;
-                }
-                throw new RuntimeException(ex);
-            }
-        }
-    }
 }
diff --git a/core/java/com/android/internal/os/ZygoteConnection.java b/core/java/com/android/internal/os/ZygoteConnection.java
index f0c0b99..33382ed 100644
--- a/core/java/com/android/internal/os/ZygoteConnection.java
+++ b/core/java/com/android/internal/os/ZygoteConnection.java
@@ -26,7 +26,6 @@
 import android.net.LocalSocket;
 import android.os.FactoryTest;
 import android.os.Process;
-import android.os.SELinux;
 import android.os.SystemProperties;
 import android.os.Trace;
 import android.system.ErrnoException;
@@ -36,15 +35,13 @@
 import java.io.BufferedReader;
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
+import java.io.EOFException;
 import java.io.FileDescriptor;
 import java.io.FileInputStream;
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
-import java.io.PrintStream;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
-import java.util.Arrays;
 import libcore.io.IoUtils;
 
 /**
@@ -80,6 +77,7 @@
     private final BufferedReader mSocketReader;
     private final Credentials peer;
     private final String abiList;
+    private boolean isEof;
 
     /**
      * Constructs instance from connected socket.
@@ -106,6 +104,8 @@
             Log.e(TAG, "Cannot read peer credentials", ex);
             throw ex;
         }
+
+        isEof = false;
     }
 
     /**
@@ -118,21 +118,14 @@
     }
 
     /**
-     * Reads one start command from the command socket. If successful,
-     * a child is forked and a {@link Zygote.MethodAndArgsCaller}
-     * exception is thrown in that child while in the parent process,
-     * the method returns normally. On failure, the child is not
-     * spawned and messages are printed to the log and stderr. Returns
-     * a boolean status value indicating whether an end-of-file on the command
-     * socket has been encountered.
+     * Reads one start command from the command socket. If successful, a child is forked and a
+     * {@code Runnable} that calls the childs main method (or equivalent) is returned in the child
+     * process. {@code null} is always returned in the parent process (the zygote).
      *
-     * @return false if command socket should continue to be read from, or
-     * true if an end-of-file has been encountered.
-     * @throws Zygote.MethodAndArgsCaller trampoline to invoke main()
-     * method in child process
+     * If the client closes the socket, an {@code EOF} condition is set, which callers can test
+     * for by calling {@code ZygoteConnection.isClosedByPeer}.
      */
-    boolean runOnce(ZygoteServer zygoteServer) throws Zygote.MethodAndArgsCaller {
-
+    Runnable processOneCommand(ZygoteServer zygoteServer) {
         String args[];
         Arguments parsedArgs = null;
         FileDescriptor[] descriptors;
@@ -141,130 +134,120 @@
             args = readArgumentList();
             descriptors = mSocket.getAncillaryFileDescriptors();
         } catch (IOException ex) {
-            Log.w(TAG, "IOException on command socket " + ex.getMessage());
-            closeSocket();
-            return true;
+            throw new IllegalStateException("IOException on command socket", ex);
         }
 
+        // readArgumentList returns null only when it has reached EOF with no available
+        // data to read. This will only happen when the remote socket has disconnected.
         if (args == null) {
-            // EOF reached.
-            closeSocket();
-            return true;
-        }
-
-        /** the stderr of the most recent request, if avail */
-        PrintStream newStderr = null;
-
-        if (descriptors != null && descriptors.length >= 3) {
-            newStderr = new PrintStream(
-                    new FileOutputStream(descriptors[2]));
+            isEof = true;
+            return null;
         }
 
         int pid = -1;
         FileDescriptor childPipeFd = null;
         FileDescriptor serverPipeFd = null;
 
-        try {
-            parsedArgs = new Arguments(args);
+        parsedArgs = new Arguments(args);
 
-            if (parsedArgs.abiListQuery) {
-                return handleAbiListQuery();
-            }
+        if (parsedArgs.abiListQuery) {
+            handleAbiListQuery();
+            return null;
+        }
 
-            if (parsedArgs.preloadDefault) {
-                return handlePreload();
-            }
+        if (parsedArgs.preloadDefault) {
+            handlePreload();
+            return null;
+        }
 
-            if (parsedArgs.preloadPackage != null) {
-                return handlePreloadPackage(parsedArgs.preloadPackage,
-                        parsedArgs.preloadPackageLibs, parsedArgs.preloadPackageCacheKey);
-            }
+        if (parsedArgs.preloadPackage != null) {
+            handlePreloadPackage(parsedArgs.preloadPackage, parsedArgs.preloadPackageLibs,
+                    parsedArgs.preloadPackageCacheKey);
+            return null;
+        }
 
-            if (parsedArgs.permittedCapabilities != 0 || parsedArgs.effectiveCapabilities != 0) {
-                throw new ZygoteSecurityException("Client may not specify capabilities: " +
-                        "permitted=0x" + Long.toHexString(parsedArgs.permittedCapabilities) +
-                        ", effective=0x" + Long.toHexString(parsedArgs.effectiveCapabilities));
-            }
+        if (parsedArgs.permittedCapabilities != 0 || parsedArgs.effectiveCapabilities != 0) {
+            throw new ZygoteSecurityException("Client may not specify capabilities: " +
+                    "permitted=0x" + Long.toHexString(parsedArgs.permittedCapabilities) +
+                    ", effective=0x" + Long.toHexString(parsedArgs.effectiveCapabilities));
+        }
 
-            applyUidSecurityPolicy(parsedArgs, peer);
-            applyInvokeWithSecurityPolicy(parsedArgs, peer);
+        applyUidSecurityPolicy(parsedArgs, peer);
+        applyInvokeWithSecurityPolicy(parsedArgs, peer);
 
-            applyDebuggerSystemProperty(parsedArgs);
-            applyInvokeWithSystemProperty(parsedArgs);
+        applyDebuggerSystemProperty(parsedArgs);
+        applyInvokeWithSystemProperty(parsedArgs);
 
-            int[][] rlimits = null;
+        int[][] rlimits = null;
 
-            if (parsedArgs.rlimits != null) {
-                rlimits = parsedArgs.rlimits.toArray(intArray2d);
-            }
+        if (parsedArgs.rlimits != null) {
+            rlimits = parsedArgs.rlimits.toArray(intArray2d);
+        }
 
-            int[] fdsToIgnore = null;
+        int[] fdsToIgnore = null;
 
-            if (parsedArgs.invokeWith != null) {
+        if (parsedArgs.invokeWith != null) {
+            try {
                 FileDescriptor[] pipeFds = Os.pipe2(O_CLOEXEC);
                 childPipeFd = pipeFds[1];
                 serverPipeFd = pipeFds[0];
                 Os.fcntlInt(childPipeFd, F_SETFD, 0);
-                fdsToIgnore = new int[] { childPipeFd.getInt$(), serverPipeFd.getInt$() };
+                fdsToIgnore = new int[]{childPipeFd.getInt$(), serverPipeFd.getInt$()};
+            } catch (ErrnoException errnoEx) {
+                throw new IllegalStateException("Unable to set up pipe for invoke-with", errnoEx);
             }
-
-            /**
-             * In order to avoid leaking descriptors to the Zygote child,
-             * the native code must close the two Zygote socket descriptors
-             * in the child process before it switches from Zygote-root to
-             * the UID and privileges of the application being launched.
-             *
-             * In order to avoid "bad file descriptor" errors when the
-             * two LocalSocket objects are closed, the Posix file
-             * descriptors are released via a dup2() call which closes
-             * the socket and substitutes an open descriptor to /dev/null.
-             */
-
-            int [] fdsToClose = { -1, -1 };
-
-            FileDescriptor fd = mSocket.getFileDescriptor();
-
-            if (fd != null) {
-                fdsToClose[0] = fd.getInt$();
-            }
-
-            fd = zygoteServer.getServerSocketFileDescriptor();
-
-            if (fd != null) {
-                fdsToClose[1] = fd.getInt$();
-            }
-
-            fd = null;
-
-            pid = Zygote.forkAndSpecialize(parsedArgs.uid, parsedArgs.gid, parsedArgs.gids,
-                    parsedArgs.debugFlags, rlimits, parsedArgs.mountExternal, parsedArgs.seInfo,
-                    parsedArgs.niceName, fdsToClose, fdsToIgnore, parsedArgs.instructionSet,
-                    parsedArgs.appDataDir);
-        } catch (ErrnoException ex) {
-            logAndPrintError(newStderr, "Exception creating pipe", ex);
-        } catch (IllegalArgumentException ex) {
-            logAndPrintError(newStderr, "Invalid zygote arguments", ex);
-        } catch (ZygoteSecurityException ex) {
-            logAndPrintError(newStderr,
-                    "Zygote security policy prevents request: ", ex);
         }
 
+        /**
+         * In order to avoid leaking descriptors to the Zygote child,
+         * the native code must close the two Zygote socket descriptors
+         * in the child process before it switches from Zygote-root to
+         * the UID and privileges of the application being launched.
+         *
+         * In order to avoid "bad file descriptor" errors when the
+         * two LocalSocket objects are closed, the Posix file
+         * descriptors are released via a dup2() call which closes
+         * the socket and substitutes an open descriptor to /dev/null.
+         */
+
+        int [] fdsToClose = { -1, -1 };
+
+        FileDescriptor fd = mSocket.getFileDescriptor();
+
+        if (fd != null) {
+            fdsToClose[0] = fd.getInt$();
+        }
+
+        fd = zygoteServer.getServerSocketFileDescriptor();
+
+        if (fd != null) {
+            fdsToClose[1] = fd.getInt$();
+        }
+
+        fd = null;
+
+        pid = Zygote.forkAndSpecialize(parsedArgs.uid, parsedArgs.gid, parsedArgs.gids,
+                parsedArgs.debugFlags, rlimits, parsedArgs.mountExternal, parsedArgs.seInfo,
+                parsedArgs.niceName, fdsToClose, fdsToIgnore, parsedArgs.instructionSet,
+                parsedArgs.appDataDir);
+
         try {
             if (pid == 0) {
                 // in child
+                zygoteServer.setForkChild();
+
                 zygoteServer.closeServerSocket();
                 IoUtils.closeQuietly(serverPipeFd);
                 serverPipeFd = null;
-                handleChildProc(parsedArgs, descriptors, childPipeFd, newStderr);
 
-                // should never get here, the child is expected to either
-                // throw Zygote.MethodAndArgsCaller or exec().
-                return true;
+                return handleChildProc(parsedArgs, descriptors, childPipeFd);
             } else {
-                // in parent...pid of < 0 means failure
+                // In the parent. A pid < 0 indicates a failure and will be handled in
+                // handleParentProc.
                 IoUtils.closeQuietly(childPipeFd);
                 childPipeFd = null;
-                return handleParentProc(pid, descriptors, serverPipeFd, parsedArgs);
+                handleParentProc(pid, descriptors, serverPipeFd);
+                return null;
             }
         } finally {
             IoUtils.closeQuietly(childPipeFd);
@@ -272,15 +255,13 @@
         }
     }
 
-    private boolean handleAbiListQuery() {
+    private void handleAbiListQuery() {
         try {
             final byte[] abiListBytes = abiList.getBytes(StandardCharsets.US_ASCII);
             mSocketOutStream.writeInt(abiListBytes.length);
             mSocketOutStream.write(abiListBytes);
-            return false;
         } catch (IOException ioe) {
-            Log.e(TAG, "Error writing to command socket", ioe);
-            return true;
+            throw new IllegalStateException("Error writing to command socket", ioe);
         }
     }
 
@@ -290,7 +271,7 @@
      * if no preload was initiated. The latter implies that the zygote is not configured to load
      * resources lazy or that the zygote has already handled a previous request to handlePreload.
      */
-    private boolean handlePreload() {
+    private void handlePreload() {
         try {
             if (isPreloadComplete()) {
                 mSocketOutStream.writeInt(1);
@@ -298,11 +279,8 @@
                 preload();
                 mSocketOutStream.writeInt(0);
             }
-
-            return false;
         } catch (IOException ioe) {
-            Log.e(TAG, "Error writing to command socket", ioe);
-            return true;
+            throw new IllegalStateException("Error writing to command socket", ioe);
         }
     }
 
@@ -318,7 +296,7 @@
         return mSocketOutStream;
     }
 
-    protected boolean handlePreloadPackage(String packagePath, String libsPath, String cacheKey) {
+    protected void handlePreloadPackage(String packagePath, String libsPath, String cacheKey) {
         throw new RuntimeException("Zyogte does not support package preloading");
     }
 
@@ -334,6 +312,10 @@
         }
     }
 
+    boolean isClosedByPeer() {
+        return isEof;
+    }
+
     /**
      * Handles argument parsing for args related to the zygote spawner.
      *
@@ -781,15 +763,9 @@
      * @param parsedArgs non-null; zygote args
      * @param descriptors null-ok; new file descriptors for stdio if available.
      * @param pipeFd null-ok; pipe for communication back to Zygote.
-     * @param newStderr null-ok; stream to use for stderr until stdio
-     * is reopened.
-     *
-     * @throws Zygote.MethodAndArgsCaller on success to
-     * trampoline to code that invokes static main.
      */
-    private void handleChildProc(Arguments parsedArgs,
-            FileDescriptor[] descriptors, FileDescriptor pipeFd, PrintStream newStderr)
-            throws Zygote.MethodAndArgsCaller {
+    private Runnable handleChildProc(Arguments parsedArgs, FileDescriptor[] descriptors,
+            FileDescriptor pipeFd) {
         /**
          * By the time we get here, the native code has closed the two actual Zygote
          * socket connections, and substituted /dev/null in their place.  The LocalSocket
@@ -806,7 +782,6 @@
                 for (FileDescriptor fd: descriptors) {
                     IoUtils.closeQuietly(fd);
                 }
-                newStderr = System.err;
             } catch (ErrnoException ex) {
                 Log.e(TAG, "Error reopening stdio", ex);
             }
@@ -823,9 +798,12 @@
                     parsedArgs.niceName, parsedArgs.targetSdkVersion,
                     VMRuntime.getCurrentInstructionSet(),
                     pipeFd, parsedArgs.remainingArgs);
+
+            // Should not get here.
+            throw new IllegalStateException("WrapperInit.execApplication unexpectedly returned");
         } else {
-            ZygoteInit.zygoteInit(parsedArgs.targetSdkVersion,
-                    parsedArgs.remainingArgs, null /* classLoader */);
+            return ZygoteInit.zygoteInit(parsedArgs.targetSdkVersion, parsedArgs.remainingArgs,
+                    null /* classLoader */);
         }
     }
 
@@ -837,13 +815,8 @@
      * @param descriptors null-ok; file descriptors for child's new stdio if
      * specified.
      * @param pipeFd null-ok; pipe for communication with child.
-     * @param parsedArgs non-null; zygote args
-     * @return true for "exit command loop" and false for "continue command
-     * loop"
      */
-    private boolean handleParentProc(int pid,
-            FileDescriptor[] descriptors, FileDescriptor pipeFd, Arguments parsedArgs) {
-
+    private void handleParentProc(int pid, FileDescriptor[] descriptors, FileDescriptor pipeFd) {
         if (pid > 0) {
             setChildPgid(pid);
         }
@@ -892,11 +865,8 @@
             mSocketOutStream.writeInt(pid);
             mSocketOutStream.writeBoolean(usingWrapper);
         } catch (IOException ex) {
-            Log.e(TAG, "Error writing to command socket", ex);
-            return true;
+            throw new IllegalStateException("Error writing to command socket", ex);
         }
-
-        return false;
     }
 
     private void setChildPgid(int pid) {
@@ -912,20 +882,4 @@
                 + "normal if peer is not in our session");
         }
     }
-
-    /**
-     * Logs an error message and prints it to the specified stream, if
-     * provided
-     *
-     * @param newStderr null-ok; a standard error stream
-     * @param message non-null; error message
-     * @param ex null-ok an exception
-     */
-    private static void logAndPrintError (PrintStream newStderr,
-            String message, Throwable ex) {
-        Log.e(TAG, message, ex);
-        if (newStderr != null) {
-            newStderr.println(message + (ex == null ? "" : ex));
-        }
-    }
 }
diff --git a/core/java/com/android/internal/os/ZygoteInit.java b/core/java/com/android/internal/os/ZygoteInit.java
index 385976c..b9022bc 100644
--- a/core/java/com/android/internal/os/ZygoteInit.java
+++ b/core/java/com/android/internal/os/ZygoteInit.java
@@ -24,7 +24,6 @@
 import android.icu.impl.CacheValue;
 import android.icu.text.DecimalFormatSymbols;
 import android.icu.util.ULocale;
-import android.net.LocalServerSocket;
 import android.opengl.EGL14;
 import android.os.Build;
 import android.os.IInstalld;
@@ -446,10 +445,7 @@
     /**
      * Finish remaining work for the newly forked system server process.
      */
-    private static void handleSystemServerProcess(
-            ZygoteConnection.Arguments parsedArgs)
-            throws Zygote.MethodAndArgsCaller {
-
+    private static Runnable handleSystemServerProcess(ZygoteConnection.Arguments parsedArgs) {
         // set umask to 0077 so new files and directories will default to owner-only permissions.
         Os.umask(S_IRWXG | S_IRWXO);
 
@@ -495,6 +491,8 @@
             WrapperInit.execApplication(parsedArgs.invokeWith,
                     parsedArgs.niceName, parsedArgs.targetSdkVersion,
                     VMRuntime.getCurrentInstructionSet(), null, args);
+
+            throw new IllegalStateException("Unexpected return from WrapperInit.execApplication");
         } else {
             ClassLoader cl = null;
             if (systemServerClasspath != null) {
@@ -506,7 +504,7 @@
             /*
              * Pass the remaining arguments to SystemServer.
              */
-            ZygoteInit.zygoteInit(parsedArgs.targetSdkVersion, parsedArgs.remainingArgs, cl);
+            return ZygoteInit.zygoteInit(parsedArgs.targetSdkVersion, parsedArgs.remainingArgs, cl);
         }
 
         /* should never reach here */
@@ -588,10 +586,13 @@
     }
 
     /**
-     * Prepare the arguments and fork for the system server process.
+     * Prepare the arguments and forks for the system server process.
+     *
+     * Returns an {@code Runnable} that provides an entrypoint into system_server code in the
+     * child process, and {@code null} in the parent.
      */
-    private static boolean startSystemServer(String abiList, String socketName, ZygoteServer zygoteServer)
-            throws Zygote.MethodAndArgsCaller, RuntimeException {
+    private static Runnable forkSystemServer(String abiList, String socketName,
+            ZygoteServer zygoteServer) {
         long capabilities = posixCapabilitiesAsBits(
             OsConstants.CAP_IPC_LOCK,
             OsConstants.CAP_KILL,
@@ -648,10 +649,10 @@
             }
 
             zygoteServer.closeServerSocket();
-            handleSystemServerProcess(parsedArgs);
+            return handleSystemServerProcess(parsedArgs);
         }
 
-        return true;
+        return null;
     }
 
     /**
@@ -682,6 +683,7 @@
             throw new RuntimeException("Failed to setpgid(0,0)", ex);
         }
 
+        final Runnable caller;
         try {
             // Report Zygote start time to tron unless it is a runtime restart
             if (!"1".equals(SystemProperties.get("sys.boot_completed"))) {
@@ -751,19 +753,32 @@
             ZygoteHooks.stopZygoteNoThreadCreation();
 
             if (startSystemServer) {
-                startSystemServer(abiList, socketName, zygoteServer);
+                Runnable r = forkSystemServer(abiList, socketName, zygoteServer);
+
+                // {@code r == null} in the parent (zygote) process, and {@code r != null} in the
+                // child (system_server) process.
+                if (r != null) {
+                    r.run();
+                    return;
+                }
             }
 
             Log.i(TAG, "Accepting command socket connections");
-            zygoteServer.runSelectLoop(abiList);
 
-            zygoteServer.closeServerSocket();
-        } catch (Zygote.MethodAndArgsCaller caller) {
-            caller.run();
+            // The select loop returns early in the child process after a fork and
+            // loops forever in the zygote.
+            caller = zygoteServer.runSelectLoop(abiList);
         } catch (Throwable ex) {
             Log.e(TAG, "System zygote died with exception", ex);
-            zygoteServer.closeServerSocket();
             throw ex;
+        } finally {
+            zygoteServer.closeServerSocket();
+        }
+
+        // We're in the child process and have exited the select loop. Proceed to execute the
+        // command.
+        if (caller != null) {
+            caller.run();
         }
     }
 
@@ -807,8 +822,7 @@
      * @param targetSdkVersion target SDK version
      * @param argv arg strings
      */
-    public static final void zygoteInit(int targetSdkVersion, String[] argv,
-            ClassLoader classLoader) throws Zygote.MethodAndArgsCaller {
+    public static final Runnable zygoteInit(int targetSdkVersion, String[] argv, ClassLoader classLoader) {
         if (RuntimeInit.DEBUG) {
             Slog.d(RuntimeInit.TAG, "RuntimeInit: Starting application from zygote");
         }
@@ -818,7 +832,7 @@
 
         RuntimeInit.commonInit();
         ZygoteInit.nativeZygoteInit();
-        RuntimeInit.applicationInit(targetSdkVersion, argv, classLoader);
+        return RuntimeInit.applicationInit(targetSdkVersion, argv, classLoader);
     }
 
     private static final native void nativeZygoteInit();
diff --git a/core/java/com/android/internal/os/ZygoteServer.java b/core/java/com/android/internal/os/ZygoteServer.java
index 126d9e7..8baa15a 100644
--- a/core/java/com/android/internal/os/ZygoteServer.java
+++ b/core/java/com/android/internal/os/ZygoteServer.java
@@ -25,6 +25,7 @@
 import android.system.StructPollfd;
 import android.util.Log;
 
+import android.util.Slog;
 import java.io.IOException;
 import java.io.FileDescriptor;
 import java.util.ArrayList;
@@ -45,9 +46,18 @@
 
     private LocalServerSocket mServerSocket;
 
+    /**
+     * Set by the child process, immediately after a call to {@code Zygote.forkAndSpecialize}.
+     */
+    private boolean mIsForkChild;
+
     ZygoteServer() {
     }
 
+    void setForkChild() {
+        mIsForkChild = true;
+    }
+
     /**
      * Registers a server socket for zygote command connections
      *
@@ -129,11 +139,8 @@
      * Runs the zygote process's select loop. Accepts new connections as
      * they happen, and reads commands from connections one spawn-request's
      * worth at a time.
-     *
-     * @throws Zygote.MethodAndArgsCaller in a child process when a main()
-     * should be executed.
      */
-    void runSelectLoop(String abiList) throws Zygote.MethodAndArgsCaller {
+    Runnable runSelectLoop(String abiList) {
         ArrayList<FileDescriptor> fds = new ArrayList<FileDescriptor>();
         ArrayList<ZygoteConnection> peers = new ArrayList<ZygoteConnection>();
 
@@ -156,15 +163,62 @@
                 if ((pollFds[i].revents & POLLIN) == 0) {
                     continue;
                 }
+
                 if (i == 0) {
                     ZygoteConnection newPeer = acceptCommandPeer(abiList);
                     peers.add(newPeer);
                     fds.add(newPeer.getFileDesciptor());
                 } else {
-                    boolean done = peers.get(i).runOnce(this);
-                    if (done) {
-                        peers.remove(i);
-                        fds.remove(i);
+                    try {
+                        ZygoteConnection connection = peers.get(i);
+                        final Runnable command = connection.processOneCommand(this);
+
+                        if (mIsForkChild) {
+                            // We're in the child. We should always have a command to run at this
+                            // stage if processOneCommand hasn't called "exec".
+                            if (command == null) {
+                                throw new IllegalStateException("command == null");
+                            }
+
+                            return command;
+                        } else {
+                            // We're in the server - we should never have any commands to run.
+                            if (command != null) {
+                                throw new IllegalStateException("command != null");
+                            }
+
+                            // We don't know whether the remote side of the socket was closed or
+                            // not until we attempt to read from it from processOneCommand. This shows up as
+                            // a regular POLLIN event in our regular processing loop.
+                            if (connection.isClosedByPeer()) {
+                                connection.closeSocket();
+                                peers.remove(i);
+                                fds.remove(i);
+                            }
+                        }
+                    } catch (Exception e) {
+                        if (!mIsForkChild) {
+                            // We're in the server so any exception here is one that has taken place
+                            // pre-fork while processing commands or reading / writing from the
+                            // control socket. Make a loud noise about any such exceptions so that
+                            // we know exactly what failed and why.
+
+                            Slog.e(TAG, "Exception executing zygote command: ", e);
+
+                            // Make sure the socket is closed so that the other end knows immediately
+                            // that something has gone wrong and doesn't time out waiting for a
+                            // response.
+                            ZygoteConnection conn = peers.remove(i);
+                            conn.closeSocket();
+
+                            fds.remove(i);
+                        } else {
+                            // We're in the child so any exception caught here has happened post
+                            // fork and before we execute ActivityThread.main (or any other main()
+                            // method). Log the details of the exception and bring down the process.
+                            Log.e(TAG, "Caught post-fork exception in child process.", e);
+                            throw e;
+                        }
                     }
                 }
             }