Move the waiting for the publishing of content provider to client side
...except the external provider client.
Previously the waiting is within the system_server and holds a binder
thread of the system_server. Now the waiting will be in client side
and the system_server will notify the client when the provider is
ready.
Bug: 149935749
Bug: 162450085
Test: atest CtsContentTestCases:android.content.cts
Test: atest FrameworksCoreTests:android.content
Test: atest ActivityManagerPerfTests:ContentProviderPerfTest
Change-Id: I0c8ef1c397103ac38f2fb7da79e647c0b8cd63cc
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index 9b13d25..dde0be5 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -421,9 +421,14 @@
final String authority;
final int userId;
+ @GuardedBy("mLock")
+ ContentProviderHolder mHolder; // Temp holder to be used between notifier and waiter
+ Object mLock; // The lock to be used to get notified when the provider is ready
+
public ProviderKey(String authority, int userId) {
this.authority = authority;
this.userId = userId;
+ this.mLock = new Object();
}
@Override
@@ -437,7 +442,11 @@
@Override
public int hashCode() {
- return ((authority != null) ? authority.hashCode() : 0) ^ userId;
+ return hashCode(authority, userId);
+ }
+
+ public static int hashCode(final String auth, final int userIdent) {
+ return ((auth != null) ? auth.hashCode() : 0) ^ userIdent;
}
}
@@ -458,9 +467,8 @@
// Mitigation for b/74523247: Used to serialize calls to AM.getContentProvider().
// Note we never removes items from this map but that's okay because there are only so many
// users and so many authorities.
- // TODO Remove it once we move CPR.wait() from AMS to the client side.
- @GuardedBy("mGetProviderLocks")
- final ArrayMap<ProviderKey, Object> mGetProviderLocks = new ArrayMap<>();
+ @GuardedBy("mGetProviderKeys")
+ final SparseArray<ProviderKey> mGetProviderKeys = new SparseArray<>();
final ArrayMap<Activity, ArrayList<OnActivityPausedListener>> mOnPauseListeners
= new ArrayMap<Activity, ArrayList<OnActivityPausedListener>>();
@@ -1751,6 +1759,16 @@
ActivityThread.this, activityToken, actionId, arguments,
cancellationSignal, resultCallback));
}
+
+ @Override
+ public void notifyContentProviderPublishStatus(@NonNull ContentProviderHolder holder,
+ @NonNull String auth, int userId, boolean published) {
+ final ProviderKey key = getGetProviderKey(auth, userId);
+ synchronized (key.mLock) {
+ key.mHolder = holder;
+ key.mLock.notifyAll();
+ }
+ }
}
private @NonNull SafeCancellationTransport createSafeCancellationTransport(
@@ -6796,13 +6814,33 @@
// provider since it might take a long time to run and it could also potentially
// be re-entrant in the case where the provider is in the same process.
ContentProviderHolder holder = null;
+ final ProviderKey key = getGetProviderKey(auth, userId);
try {
- synchronized (getGetProviderLock(auth, userId)) {
+ synchronized (key) {
holder = ActivityManager.getService().getContentProvider(
getApplicationThread(), c.getOpPackageName(), auth, userId, stable);
+ // If the returned holder is non-null but its provider is null and it's not
+ // local, we'll need to wait for the publishing of the provider.
+ if (holder != null && holder.provider == null && !holder.mLocal) {
+ synchronized (key.mLock) {
+ key.mLock.wait(ContentResolver.CONTENT_PROVIDER_READY_TIMEOUT_MILLIS);
+ holder = key.mHolder;
+ }
+ if (holder != null && holder.provider == null) {
+ // probably timed out
+ holder = null;
+ }
+ }
}
} catch (RemoteException ex) {
throw ex.rethrowFromSystemServer();
+ } catch (InterruptedException e) {
+ holder = null;
+ } finally {
+ // Clear the holder from the key since the key itself is never cleared.
+ synchronized (key.mLock) {
+ key.mHolder = null;
+ }
}
if (holder == null) {
if (UserManager.get(c).isUserUnlocked(userId)) {
@@ -6820,13 +6858,13 @@
return holder.provider;
}
- private Object getGetProviderLock(String auth, int userId) {
- final ProviderKey key = new ProviderKey(auth, userId);
- synchronized (mGetProviderLocks) {
- Object lock = mGetProviderLocks.get(key);
+ private ProviderKey getGetProviderKey(String auth, int userId) {
+ final int key = ProviderKey.hashCode(auth, userId);
+ synchronized (mGetProviderKeys) {
+ ProviderKey lock = mGetProviderKeys.get(key);
if (lock == null) {
- lock = key;
- mGetProviderLocks.put(key, lock);
+ lock = new ProviderKey(auth, userId);
+ mGetProviderKeys.put(key, lock);
}
return lock;
}
diff --git a/core/java/android/app/ContentProviderHolder.java b/core/java/android/app/ContentProviderHolder.java
index 3d74583..e330a30 100644
--- a/core/java/android/app/ContentProviderHolder.java
+++ b/core/java/android/app/ContentProviderHolder.java
@@ -39,6 +39,11 @@
@UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
public boolean noReleaseNeeded;
+ /**
+ * Whether the provider here is a local provider or not.
+ */
+ public boolean mLocal;
+
@UnsupportedAppUsage
public ContentProviderHolder(ProviderInfo _info) {
info = _info;
@@ -59,6 +64,7 @@
}
dest.writeStrongBinder(connection);
dest.writeInt(noReleaseNeeded ? 1 : 0);
+ dest.writeInt(mLocal ? 1 : 0);
}
public static final @android.annotation.NonNull Parcelable.Creator<ContentProviderHolder> CREATOR
@@ -81,5 +87,6 @@
source.readStrongBinder());
connection = source.readStrongBinder();
noReleaseNeeded = source.readInt() != 0;
+ mLocal = source.readInt() != 0;
}
-}
\ No newline at end of file
+}
diff --git a/core/java/android/app/IApplicationThread.aidl b/core/java/android/app/IApplicationThread.aidl
index 6e9157e..e5d4a76 100644
--- a/core/java/android/app/IApplicationThread.aidl
+++ b/core/java/android/app/IApplicationThread.aidl
@@ -16,6 +16,7 @@
package android.app;
+import android.app.ContentProviderHolder;
import android.app.IInstrumentationWatcher;
import android.app.IUiAutomationConnection;
import android.app.ProfilerInfo;
@@ -147,4 +148,6 @@
void performDirectAction(IBinder activityToken, String actionId,
in Bundle arguments, in RemoteCallback cancellationCallback,
in RemoteCallback resultCallback);
+ void notifyContentProviderPublishStatus(in ContentProviderHolder holder, String auth,
+ int userId, boolean published);
}
diff --git a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java
index f11adef..ea8df83 100644
--- a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java
+++ b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java
@@ -24,6 +24,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import android.app.ContentProviderHolder;
import android.app.IApplicationThread;
import android.app.IInstrumentationWatcher;
import android.app.IUiAutomationConnection;
@@ -664,5 +665,10 @@
public void performDirectAction(IBinder activityToken, String actionId, Bundle arguments,
RemoteCallback cancellationCallback, RemoteCallback resultCallback) {
}
+
+ @Override
+ public void notifyContentProviderPublishStatus(ContentProviderHolder holder, String auth,
+ int userId, boolean published) {
+ }
}
}
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 91a1487..5555650 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -1579,6 +1579,7 @@
static final int DISPATCH_OOM_ADJ_OBSERVER_MSG = 70;
static final int KILL_APP_ZYGOTE_MSG = 71;
static final int BINDER_HEAVYHITTER_AUTOSAMPLER_TIMEOUT_MSG = 72;
+ static final int WAIT_FOR_CONTENT_PROVIDER_TIMEOUT_MSG = 73;
static final int FIRST_BROADCAST_QUEUE_MSG = 200;
@@ -1915,6 +1916,11 @@
case BINDER_HEAVYHITTER_AUTOSAMPLER_TIMEOUT_MSG: {
handleBinderHeavyHitterAutoSamplerTimeOut();
} break;
+ case WAIT_FOR_CONTENT_PROVIDER_TIMEOUT_MSG: {
+ synchronized (ActivityManagerService.this) {
+ ((ContentProviderRecord) msg.obj).onProviderPublishStatusLocked(false);
+ }
+ } break;
}
}
}
diff --git a/services/core/java/com/android/server/am/ContentProviderHelper.java b/services/core/java/com/android/server/am/ContentProviderHelper.java
index 5cc7aba..bfba4af 100644
--- a/services/core/java/com/android/server/am/ContentProviderHelper.java
+++ b/services/core/java/com/android/server/am/ContentProviderHelper.java
@@ -50,6 +50,7 @@
import android.os.Bundle;
import android.os.Debug;
import android.os.IBinder;
+import android.os.Message;
import android.os.Process;
import android.os.RemoteCallback;
import android.os.RemoteException;
@@ -218,7 +219,7 @@
// of being published... but it is also allowed to run
// in the caller's process, so don't make a connection
// and just let the caller instantiate its own instance.
- ContentProviderHolder holder = cpr.newHolder(null);
+ ContentProviderHolder holder = cpr.newHolder(null, true);
// don't give caller the provider object, it needs to make its own.
holder.provider = null;
return holder;
@@ -415,7 +416,7 @@
// info and allow the caller to instantiate it. Only do
// this if the provider is the same user as the caller's
// process, or can run as root (so can be in any process).
- return cpr.newHolder(null);
+ return cpr.newHolder(null, true);
}
if (ActivityManagerDebugConfig.DEBUG_PROVIDER) {
@@ -513,6 +514,38 @@
UserHandle.getAppId(cpi.applicationInfo.uid));
}
+ if (caller != null) {
+ // The client will be waiting, and we'll notify it when the provider is ready.
+ synchronized (cpr) {
+ if (cpr.provider == null) {
+ if (cpr.launchingApp == null) {
+ Slog.w(TAG, "Unable to launch app "
+ + cpi.applicationInfo.packageName + "/"
+ + cpi.applicationInfo.uid + " for provider "
+ + name + ": launching app became null");
+ EventLogTags.writeAmProviderLostProcess(
+ UserHandle.getUserId(cpi.applicationInfo.uid),
+ cpi.applicationInfo.packageName,
+ cpi.applicationInfo.uid, name);
+ return null;
+ }
+
+ if (conn != null) {
+ conn.waiting = true;
+ }
+ Message msg = mService.mHandler.obtainMessage(
+ ActivityManagerService.WAIT_FOR_CONTENT_PROVIDER_TIMEOUT_MSG);
+ msg.obj = cpr;
+ mService.mHandler.sendMessageDelayed(msg,
+ ContentResolver.CONTENT_PROVIDER_READY_TIMEOUT_MILLIS);
+ }
+ }
+ // Return a holder instance even if we are waiting for the publishing of the provider,
+ // client will check for the holder.provider to see if it needs to wait for it.
+ return cpr.newHolder(conn, false);
+ }
+
+ // Because of the provider's external client (i.e., SHELL), we'll have to wait right here.
// Wait for the provider to be published...
final long timeout =
SystemClock.uptimeMillis() + ContentResolver.CONTENT_PROVIDER_READY_TIMEOUT_MILLIS;
@@ -569,7 +602,7 @@
+ " caller=" + callerName + "/" + Binder.getCallingUid());
return null;
}
- return cpr.newHolder(conn);
+ return cpr.newHolder(conn, false);
}
void publishContentProviders(IApplicationThread caller, List<ContentProviderHolder> providers) {
@@ -622,6 +655,8 @@
}
if (wasInLaunchingProviders) {
mService.mHandler.removeMessages(
+ ActivityManagerService.WAIT_FOR_CONTENT_PROVIDER_TIMEOUT_MSG, dst);
+ mService.mHandler.removeMessages(
ActivityManagerService.CONTENT_PROVIDER_PUBLISH_TIMEOUT_MSG, r);
}
// Make sure the package is associated with the process.
@@ -635,6 +670,7 @@
dst.provider = src.provider;
dst.setProcess(r);
dst.notifyAll();
+ dst.onProviderPublishStatusLocked(true);
}
dst.mRestartCount = 0;
mService.updateOomAdjLocked(r, true, OomAdjuster.OOM_ADJ_REASON_GET_PROVIDER);
@@ -1504,6 +1540,9 @@
synchronized (cpr) {
cpr.launchingApp = null;
cpr.notifyAll();
+ cpr.onProviderPublishStatusLocked(false);
+ mService.mHandler.removeMessages(
+ ActivityManagerService.WAIT_FOR_CONTENT_PROVIDER_TIMEOUT_MSG, cpr);
}
final int userId = UserHandle.getUserId(cpr.uid);
// Don't remove from provider map if it doesn't match
diff --git a/services/core/java/com/android/server/am/ContentProviderRecord.java b/services/core/java/com/android/server/am/ContentProviderRecord.java
index d8d8ccc..fb8b5d4 100644
--- a/services/core/java/com/android/server/am/ContentProviderRecord.java
+++ b/services/core/java/com/android/server/am/ContentProviderRecord.java
@@ -85,11 +85,12 @@
noReleaseNeeded = cpr.noReleaseNeeded;
}
- public ContentProviderHolder newHolder(ContentProviderConnection conn) {
+ public ContentProviderHolder newHolder(ContentProviderConnection conn, boolean local) {
ContentProviderHolder holder = new ContentProviderHolder(info);
holder.provider = provider;
holder.noReleaseNeeded = noReleaseNeeded;
holder.connection = conn;
+ holder.mLocal = local;
return holder;
}
@@ -179,6 +180,50 @@
return !connections.isEmpty() || hasExternalProcessHandles();
}
+ /**
+ * Notify all clients that the provider has been published and ready to use,
+ * or timed out.
+ *
+ * @param status true: successfully published; false: timed out
+ */
+ void onProviderPublishStatusLocked(boolean status) {
+ final int numOfConns = connections.size();
+ final int userId = UserHandle.getUserId(appInfo.uid);
+ for (int i = 0; i < numOfConns; i++) {
+ final ContentProviderConnection conn = connections.get(i);
+ if (conn.waiting && conn.client != null) {
+ final ProcessRecord client = conn.client;
+ if (!status) {
+ if (launchingApp == null) {
+ Slog.w(TAG_AM, "Unable to launch app "
+ + appInfo.packageName + "/"
+ + appInfo.uid + " for provider "
+ + info.authority + ": launching app became null");
+ EventLogTags.writeAmProviderLostProcess(
+ userId,
+ appInfo.packageName,
+ appInfo.uid, info.authority);
+ } else {
+ Slog.wtf(TAG_AM, "Timeout waiting for provider "
+ + appInfo.packageName + "/"
+ + appInfo.uid + " for provider "
+ + info.authority
+ + " caller=" + client);
+ }
+ }
+ if (client.thread != null) {
+ try {
+ client.thread.notifyContentProviderPublishStatus(
+ newHolder(status ? conn : null, false),
+ info.authority, userId, status);
+ } catch (RemoteException e) {
+ }
+ }
+ }
+ conn.waiting = false;
+ }
+ }
+
void dump(PrintWriter pw, String prefix, boolean full) {
if (full) {
pw.print(prefix); pw.print("package=");