Revert "Move the waiting for the publishing of content provider to client side"
This reverts commit 4dc70f1c08e0c27863fae4c13343cfb47a861eaf.
Reason for revert: Droidcop-triggered revert due to breakage https://android-build.googleplex.com/builds/tests/view?invocationId=I80700005505571712&testResultId=TR62011442642947463&redirect=, bug b/163015685.
Change-Id: If85c6c12e5202b79aa197e2beddd390f8a165f22
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index 1e8d19b..9b13d25 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -420,8 +420,6 @@
private static final class ProviderKey {
final String authority;
final int userId;
- ContentProviderHolder mHolder; // Temp holder to be used between notifier and waiter
- int mWaiters; // Number of threads waiting on the publishing of the provider
public ProviderKey(String authority, int userId) {
this.authority = authority;
@@ -439,11 +437,7 @@
@Override
public int hashCode() {
- return hashCode(authority, userId);
- }
-
- public static int hashCode(final String auth, final int userIdent) {
- return ((auth != null) ? auth.hashCode() : 0) ^ userIdent;
+ return ((authority != null) ? authority.hashCode() : 0) ^ userId;
}
}
@@ -464,8 +458,9 @@
// 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.
- @GuardedBy("mGetProviderKeys")
- final SparseArray<ProviderKey> mGetProviderKeys = new SparseArray<>();
+ // TODO Remove it once we move CPR.wait() from AMS to the client side.
+ @GuardedBy("mGetProviderLocks")
+ final ArrayMap<ProviderKey, Object> mGetProviderLocks = new ArrayMap<>();
final ArrayMap<Activity, ArrayList<OnActivityPausedListener>> mOnPauseListeners
= new ArrayMap<Activity, ArrayList<OnActivityPausedListener>>();
@@ -1756,16 +1751,6 @@
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) {
- key.mHolder = holder;
- key.notifyAll();
- }
- }
}
private @NonNull SafeCancellationTransport createSafeCancellationTransport(
@@ -6811,40 +6796,13 @@
// 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);
- synchronized (key) {
- boolean wasWaiting = false;
- try {
- if (key.mWaiters == 0) {
- // No other thread is waiting for this provider, let's fetch one by ourselves.
- // 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.
- holder = ActivityManager.getService().getContentProvider(
- getApplicationThread(), c.getOpPackageName(), auth, userId, stable);
- }
- if ((holder != null && holder.provider == null && !holder.mLocal)
- || (key.mWaiters > 0 && (holder = key.mHolder) == null)) {
- try {
- key.mWaiters++;
- wasWaiting = true;
- key.wait(ContentResolver.CONTENT_PROVIDER_READY_TIMEOUT_MILLIS);
- holder = key.mHolder;
- if (holder != null && holder.provider == null) {
- // probably timed out
- holder = null;
- }
- } catch (InterruptedException e) {
- holder = null;
- }
- }
- } catch (RemoteException ex) {
- throw ex.rethrowFromSystemServer();
- } finally {
- if (wasWaiting && --key.mWaiters == 0) {
- // Clear the holder from the key since the key itself is never cleared.
- key.mHolder = null;
- }
+ try {
+ synchronized (getGetProviderLock(auth, userId)) {
+ holder = ActivityManager.getService().getContentProvider(
+ getApplicationThread(), c.getOpPackageName(), auth, userId, stable);
}
+ } catch (RemoteException ex) {
+ throw ex.rethrowFromSystemServer();
}
if (holder == null) {
if (UserManager.get(c).isUserUnlocked(userId)) {
@@ -6862,13 +6820,13 @@
return holder.provider;
}
- private ProviderKey getGetProviderKey(String auth, int userId) {
- final int key = ProviderKey.hashCode(auth, userId);
- synchronized (mGetProviderKeys) {
- ProviderKey lock = mGetProviderKeys.get(key);
+ private Object getGetProviderLock(String auth, int userId) {
+ final ProviderKey key = new ProviderKey(auth, userId);
+ synchronized (mGetProviderLocks) {
+ Object lock = mGetProviderLocks.get(key);
if (lock == null) {
- lock = new ProviderKey(auth, userId);
- mGetProviderKeys.put(key, lock);
+ lock = key;
+ mGetProviderLocks.put(key, lock);
}
return lock;
}
diff --git a/core/java/android/app/ContentProviderHolder.java b/core/java/android/app/ContentProviderHolder.java
index e330a30..3d74583 100644
--- a/core/java/android/app/ContentProviderHolder.java
+++ b/core/java/android/app/ContentProviderHolder.java
@@ -39,11 +39,6 @@
@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;
@@ -64,7 +59,6 @@
}
dest.writeStrongBinder(connection);
dest.writeInt(noReleaseNeeded ? 1 : 0);
- dest.writeInt(mLocal ? 1 : 0);
}
public static final @android.annotation.NonNull Parcelable.Creator<ContentProviderHolder> CREATOR
@@ -87,6 +81,5 @@
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 e5d4a76..6e9157e 100644
--- a/core/java/android/app/IApplicationThread.aidl
+++ b/core/java/android/app/IApplicationThread.aidl
@@ -16,7 +16,6 @@
package android.app;
-import android.app.ContentProviderHolder;
import android.app.IInstrumentationWatcher;
import android.app.IUiAutomationConnection;
import android.app.ProfilerInfo;
@@ -148,6 +147,4 @@
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 ea8df83..f11adef 100644
--- a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java
+++ b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java
@@ -24,7 +24,6 @@
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;
@@ -665,10 +664,5 @@
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 5296f75..e546a28 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -1579,7 +1579,6 @@
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;
@@ -1916,11 +1915,6 @@
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 bfba4af..5cc7aba 100644
--- a/services/core/java/com/android/server/am/ContentProviderHelper.java
+++ b/services/core/java/com/android/server/am/ContentProviderHelper.java
@@ -50,7 +50,6 @@
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;
@@ -219,7 +218,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, true);
+ ContentProviderHolder holder = cpr.newHolder(null);
// don't give caller the provider object, it needs to make its own.
holder.provider = null;
return holder;
@@ -416,7 +415,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, true);
+ return cpr.newHolder(null);
}
if (ActivityManagerDebugConfig.DEBUG_PROVIDER) {
@@ -514,38 +513,6 @@
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;
@@ -602,7 +569,7 @@
+ " caller=" + callerName + "/" + Binder.getCallingUid());
return null;
}
- return cpr.newHolder(conn, false);
+ return cpr.newHolder(conn);
}
void publishContentProviders(IApplicationThread caller, List<ContentProviderHolder> providers) {
@@ -655,8 +622,6 @@
}
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.
@@ -670,7 +635,6 @@
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);
@@ -1540,9 +1504,6 @@
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 fb8b5d4..d8d8ccc 100644
--- a/services/core/java/com/android/server/am/ContentProviderRecord.java
+++ b/services/core/java/com/android/server/am/ContentProviderRecord.java
@@ -85,12 +85,11 @@
noReleaseNeeded = cpr.noReleaseNeeded;
}
- public ContentProviderHolder newHolder(ContentProviderConnection conn, boolean local) {
+ public ContentProviderHolder newHolder(ContentProviderConnection conn) {
ContentProviderHolder holder = new ContentProviderHolder(info);
holder.provider = provider;
holder.noReleaseNeeded = noReleaseNeeded;
holder.connection = conn;
- holder.mLocal = local;
return holder;
}
@@ -180,50 +179,6 @@
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=");