From 59e477126339821ac518b698190c956e31933dc0 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Wed, 24 Oct 2018 11:39:57 -0700 Subject: [PATCH] Cherry-picking the following 3 getContentProviderImpl() CLs - http://ag/5163037 Fix a race in AMS.getContentProviderImpl(). If the provider process has just been killed by AM but appDiedLocked() hasn't been called yet, this method would wait on the wrong ContentProviderRecord. - http://ag/5201125 Fix getContentProviderImpl() permission issue - http://ag/5284403 Add timeout to AMS.getContentProviderImpl() Bug: 116876013 Bug: 110030490 Test: manual test with the following debug code: + final String prop = "debug.am.provider.fake-kill." + cpr.proc.processName; + if (SystemProperties.getBoolean(prop, false)) { + cpr.proc.kill("fake-kill", true); + SystemProperties.set(prop, "0"); + } providerRunning = !cpr.proc.killed; Then: setprop debug.am.provider.fake-kill.android.process.acore 1 And launch the contacts app when acore is already running, and make sure the timeout log shows up on logcat. Test: Also do it with work profile contacts Change-Id: I765d61a1b2719a92dff7db0162335f2c09c33421 Merged-in: I2c4ba1e87c2d47f2013befff10c49b3dc337a9a7 Merged-in: Ia35a9d36a34257873edbdcaefcf85d2373f3295e Merged-in: Id5e7fff228d777eb0d1804654600d9ea156e36fd --- .../server/am/ActivityManagerService.java | 42 +++++++++++++++++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 145c1c964a314..d1496513a3b51 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -581,6 +581,12 @@ public class ActivityManagerService extends IActivityManager.Stub // before we decide it must be hung. static final int CONTENT_PROVIDER_PUBLISH_TIMEOUT = 10*1000; + /** + * How long we wait for an provider to be published. Should be longer than + * {@link #CONTENT_PROVIDER_PUBLISH_TIMEOUT}. + */ + static final int CONTENT_PROVIDER_WAIT_TIMEOUT = 20 * 1000; + // How long we wait for a launched process to attach to the activity manager // before we decide it's never going to come up for real, when the process was // started with a wrapper for instrumentation (such as Valgrind) because it @@ -12174,6 +12180,7 @@ public class ActivityManagerService extends IActivityManager.Stub ContentProviderRecord cpr; ContentProviderConnection conn = null; ProviderInfo cpi = null; + boolean providerRunning = false; synchronized(this) { long startTime = SystemClock.uptimeMillis(); @@ -12213,7 +12220,26 @@ public class ActivityManagerService extends IActivityManager.Stub } } - boolean providerRunning = cpr != null && cpr.proc != null && !cpr.proc.killed; + if (cpr != null && cpr.proc != null) { + providerRunning = !cpr.proc.killed; + + // Note if killedByAm is also set, this means the provider process has just been + // killed by AM (in ProcessRecord.kill()), but appDiedLocked() hasn't been called + // yet. So we need to call appDiedLocked() here and let it clean up. + // (See the commit message on I2c4ba1e87c2d47f2013befff10c49b3dc337a9a7 to see + // how to test this case.) + if (cpr.proc.killed && cpr.proc.killedByAm) { + checkTime(startTime, "getContentProviderImpl: before appDied (killedByAm)"); + final long iden = Binder.clearCallingIdentity(); + try { + appDiedLocked(cpr.proc); + } finally { + Binder.restoreCallingIdentity(iden); + } + checkTime(startTime, "getContentProviderImpl: after appDied (killedByAm)"); + } + } + if (providerRunning) { cpi = cpr.info; String msg; @@ -12506,6 +12532,7 @@ public class ActivityManagerService extends IActivityManager.Stub } // Wait for the provider to be published... + final long timeout = SystemClock.uptimeMillis() + CONTENT_PROVIDER_WAIT_TIMEOUT; synchronized (cpr) { while (cpr.provider == null) { if (cpr.launchingApp == null) { @@ -12520,13 +12547,22 @@ public class ActivityManagerService extends IActivityManager.Stub return null; } try { + final long wait = Math.max(0L, timeout - SystemClock.uptimeMillis()); if (DEBUG_MU) Slog.v(TAG_MU, "Waiting to start provider " + cpr - + " launchingApp=" + cpr.launchingApp); + + " launchingApp=" + cpr.launchingApp + " for " + wait + " ms"); if (conn != null) { conn.waiting = true; } - cpr.wait(); + cpr.wait(wait); + if (cpr.provider == null) { + Slog.wtf(TAG, "Timeout waiting for provider " + + cpi.applicationInfo.packageName + "/" + + cpi.applicationInfo.uid + " for provider " + + name + + " providerRunning=" + providerRunning); + return null; + } } catch (InterruptedException ex) { } finally { if (conn != null) {