From a3d4230b92d69eb12c48f83689aeb27de992277d Mon Sep 17 00:00:00 2001 From: Narayan Kamath Date: Mon, 3 Jul 2017 14:12:26 +0100 Subject: [PATCH] Zygote: Fix race condition on package preloads. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change, ZygoteProcess.preloadPackageForAbi returned as soon as the command was written to the zygote socket and not after the preload completed. This meant that there was a small window of time before the server side of the socket polled its FDs where a second command could be written to the zygote socket. This would lead to only one of the commands being processed and the other being dropped. The client side of that socket would then wait forever for a response and bring down the system once the watchdog timeout was hit. Example failure case : -------------- system_server:send command(preloadPackage) system_server:send command(fork) zygote:poll & process command(preloadPackage) // the fork command is dropped. Example of normal operation : ------------------ system_server:send command(preloadPackage) zygote:poll & process command(preloadPackage) system_server:send command(fork) zygote:poll & process command(fork) This change makes preloadPackageForAbi synchronous, which ensures that each POLLIN event corresponds to precisely one command. (cherry picked from commit 24a3306c32aa3860184025638f3abaab96cc9153) Bug: 62886909 Bug: 13618569 Test: Manual Contributed-By: yuqianyu@huawei.com Merged-In: I83faf974c9a70a6ab18323f692c1981784e4c56a Change-Id: I83faf974c9a70a6ab18323f692c1981784e4c56a --- core/java/android/os/ZygoteProcess.java | 6 ++++-- .../com/android/internal/os/WebViewZygoteInit.java | 14 +++++++++++++- .../com/android/internal/os/ZygoteConnection.java | 4 ++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/core/java/android/os/ZygoteProcess.java b/core/java/android/os/ZygoteProcess.java index 70452006da276..93826d808c3ad 100644 --- a/core/java/android/os/ZygoteProcess.java +++ b/core/java/android/os/ZygoteProcess.java @@ -463,8 +463,8 @@ public class ZygoteProcess { * Instructs the zygote to pre-load the classes and native libraries at the given paths * for the specified abi. Not all zygotes support this function. */ - public void preloadPackageForAbi(String packagePath, String libsPath, String cacheKey, - String abi) throws ZygoteStartFailedEx, IOException { + public boolean preloadPackageForAbi(String packagePath, String libsPath, String cacheKey, + String abi) throws ZygoteStartFailedEx, IOException { synchronized(mLock) { ZygoteState state = openZygoteSocketIfNeeded(abi); state.writer.write("4"); @@ -483,6 +483,8 @@ public class ZygoteProcess { state.writer.newLine(); state.writer.flush(); + + return (state.inputStream.readInt() == 0); } } diff --git a/core/java/com/android/internal/os/WebViewZygoteInit.java b/core/java/com/android/internal/os/WebViewZygoteInit.java index e28079fd5cdd6..58e4a3ed8deb8 100644 --- a/core/java/com/android/internal/os/WebViewZygoteInit.java +++ b/core/java/com/android/internal/os/WebViewZygoteInit.java @@ -26,6 +26,7 @@ import android.util.Log; import android.webkit.WebViewFactory; import android.webkit.WebViewFactoryProvider; +import java.io.DataOutputStream; import java.io.File; import java.io.IOException; import java.lang.reflect.InvocationTargetException; @@ -87,17 +88,28 @@ class WebViewZygoteInit { // Once we have the classloader, look up the WebViewFactoryProvider implementation and // call preloadInZygote() on it to give it the opportunity to preload the native library // and perform any other initialisation work that should be shared among the children. + boolean preloadSucceeded = false; try { Class providerClass = WebViewFactory.getWebViewProviderClass(loader); Object result = providerClass.getMethod("preloadInZygote").invoke(null); - if (!((Boolean)result).booleanValue()) { + preloadSucceeded = ((Boolean) result).booleanValue(); + if (!preloadSucceeded) { Log.e(TAG, "preloadInZygote returned false"); } } catch (ClassNotFoundException | NoSuchMethodException | SecurityException | IllegalAccessException | InvocationTargetException e) { Log.e(TAG, "Exception while preloading package", e); } + + try { + DataOutputStream socketOut = getSocketOutputStream(); + socketOut.writeInt(preloadSucceeded ? 1 : 0); + } catch (IOException ioe) { + Log.e(TAG, "Error writing to command socket", ioe); + return true; + } + Log.i(TAG, "Package preload done"); return false; } diff --git a/core/java/com/android/internal/os/ZygoteConnection.java b/core/java/com/android/internal/os/ZygoteConnection.java index 45cb8405468de..d459bc31beba0 100644 --- a/core/java/com/android/internal/os/ZygoteConnection.java +++ b/core/java/com/android/internal/os/ZygoteConnection.java @@ -307,6 +307,10 @@ class ZygoteConnection { return ZygoteInit.isPreloadComplete(); } + protected DataOutputStream getSocketOutputStream() { + return mSocketOutStream; + } + protected boolean handlePreloadPackage(String packagePath, String libsPath, String cacheKey) { throw new RuntimeException("Zyogte does not support package preloading"); }