From 05a766e8764188ee50bef63d6bfb4067bdf0feac Mon Sep 17 00:00:00 2001 From: "Torne (Richard Coles)" Date: Wed, 10 Apr 2019 17:55:53 -0400 Subject: [PATCH] Wait for RELRO before starting WebViewZygote at boot. We start WebViewZygote at boot time to ensure the first app to use WebView doesn't have to wait, but we were not waiting for RELRO creation to finish before doing so. Move the background task which starts the zygote to WebViewUpdateServiceImpl and have it wait for the RELRO first to ensure that we get the memory saving of RELRO sharing whenever possible, instead of only when the timing happens to work out by chance. Fixes: 130305037 Test: manual, check logs and relro sharing status after boot Change-Id: I55c3f80b0db1dc82727b90c70f777618ca77a942 --- core/java/android/webkit/WebViewZygote.java | 15 +++------------ .../com/android/server/webkit/SystemImpl.java | 5 +++++ .../android/server/webkit/SystemInterface.java | 2 ++ .../server/webkit/WebViewUpdateServiceImpl.java | 9 +++++++++ .../com/android/server/webkit/TestSystemImpl.java | 3 +++ 5 files changed, 22 insertions(+), 12 deletions(-) diff --git a/core/java/android/webkit/WebViewZygote.java b/core/java/android/webkit/WebViewZygote.java index 62f54b943e110..2bfbe4bdfba75 100644 --- a/core/java/android/webkit/WebViewZygote.java +++ b/core/java/android/webkit/WebViewZygote.java @@ -17,7 +17,6 @@ package android.webkit; import android.content.pm.PackageInfo; -import android.os.AsyncTask; import android.os.Build; import android.os.ChildZygoteProcess; import android.os.Process; @@ -81,17 +80,9 @@ public class WebViewZygote { synchronized (sLock) { sMultiprocessEnabled = enabled; - // When toggling between multi-process being on/off, start or stop the - // zygote. If it is enabled and the zygote is not yet started, launch it. - // Otherwise, kill it. The name may be null if the package information has - // not yet been resolved. - if (enabled) { - // Run on a background thread as this waits for the zygote to start and we don't - // want to block the caller on this. It's okay if this is delayed as anyone trying - // to use the zygote will call it first anyway. - AsyncTask.THREAD_POOL_EXECUTOR.execute(WebViewZygote::getProcess); - } else { - // No need to run this in the background, it's very brief. + // When multi-process is disabled, kill the zygote. When it is enabled, + // the zygote will be started when it is first needed in getProcess(). + if (!enabled) { stopZygoteLocked(); } } diff --git a/services/core/java/com/android/server/webkit/SystemImpl.java b/services/core/java/com/android/server/webkit/SystemImpl.java index 56a6c3cb99b2e..a9a6b193fe905 100644 --- a/services/core/java/com/android/server/webkit/SystemImpl.java +++ b/services/core/java/com/android/server/webkit/SystemImpl.java @@ -265,6 +265,11 @@ public class SystemImpl implements SystemInterface { WebViewZygote.setMultiprocessEnabled(enableMultiProcess); } + @Override + public void ensureZygoteStarted() { + WebViewZygote.getProcess(); + } + @Override public boolean isMultiProcessDefaultEnabled() { // Multiprocess is enabled for all 64-bit devices, since the ability to run the renderer diff --git a/services/core/java/com/android/server/webkit/SystemInterface.java b/services/core/java/com/android/server/webkit/SystemInterface.java index 3fb52790621ac..743740d277ba2 100644 --- a/services/core/java/com/android/server/webkit/SystemInterface.java +++ b/services/core/java/com/android/server/webkit/SystemInterface.java @@ -61,5 +61,7 @@ public interface SystemInterface { public int getMultiProcessSetting(Context context); public void setMultiProcessSetting(Context context, int value); public void notifyZygote(boolean enableMultiProcess); + /** Start the zygote if it's not already running. */ + public void ensureZygoteStarted(); public boolean isMultiProcessDefaultEnabled(); } diff --git a/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java b/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java index f704c30402d32..890456aa6e9c6 100644 --- a/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java +++ b/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java @@ -17,6 +17,7 @@ package com.android.server.webkit; import android.content.Context; import android.content.pm.PackageInfo; +import android.os.AsyncTask; import android.os.UserHandle; import android.webkit.WebViewProviderInfo; import android.webkit.WebViewProviderResponse; @@ -81,6 +82,14 @@ public class WebViewUpdateServiceImpl { migrateFallbackStateOnBoot(); mWebViewUpdater.prepareWebViewInSystemServer(); mSystemInterface.notifyZygote(isMultiProcessEnabled()); + AsyncTask.THREAD_POOL_EXECUTOR.execute(this::startZygoteWhenReady); + } + + void startZygoteWhenReady() { + // Wait on a background thread for RELRO creation to be done. We ignore the return value + // because even if RELRO creation failed we still want to start the zygote. + waitForAndGetProvider(); + mSystemInterface.ensureZygoteStarted(); } void handleNewUser(int userId) { diff --git a/services/tests/servicestests/src/com/android/server/webkit/TestSystemImpl.java b/services/tests/servicestests/src/com/android/server/webkit/TestSystemImpl.java index 3f687c81ad29a..8cde10675fe3b 100644 --- a/services/tests/servicestests/src/com/android/server/webkit/TestSystemImpl.java +++ b/services/tests/servicestests/src/com/android/server/webkit/TestSystemImpl.java @@ -184,6 +184,9 @@ public class TestSystemImpl implements SystemInterface { @Override public void notifyZygote(boolean enableMultiProcess) {} + @Override + public void ensureZygoteStarted() {} + @Override public boolean isMultiProcessDefaultEnabled() { return mMultiProcessDefault;