diff options
7 files changed, 21 insertions, 169 deletions
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index 1e8d19bd59c4..9b13d256aea6 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -420,8 +420,6 @@ public final class ActivityThread extends ClientTransactionHandler { 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 @@ public final class ActivityThread extends ClientTransactionHandler { @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 @@ public final class ActivityThread extends ClientTransactionHandler { // 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 @@ public final class ActivityThread extends ClientTransactionHandler { 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 @@ public final class ActivityThread extends ClientTransactionHandler { // 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 @@ public final class ActivityThread extends ClientTransactionHandler { 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 e330a30de7b0..3d745831ce1c 100644 --- a/core/java/android/app/ContentProviderHolder.java +++ b/core/java/android/app/ContentProviderHolder.java @@ -39,11 +39,6 @@ public class ContentProviderHolder implements Parcelable { @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 @@ public class ContentProviderHolder implements Parcelable { } 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 @@ public class ContentProviderHolder implements Parcelable { 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 e5d4a766a89d..6e9157e2a8c3 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 @@ oneway interface IApplicationThread { 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 ea8df831fd24..f11adef81793 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 android.app.servertransaction.TestUtils.resultInfoList; 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 class TransactionParcelTests { 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 5555650d9be2..91a1487c5245 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 @@ public class ActivityManagerService extends IActivityManager.Stub 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 @@ public class ActivityManagerService extends IActivityManager.Stub 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 bfba4afcd4e4..5cc7aba736e1 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.Build; 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 @@ public class ContentProviderHelper { // 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 @@ public class ContentProviderHelper { // 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 @@ public class ContentProviderHelper { 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 @@ public class ContentProviderHelper { + " 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 @@ public class ContentProviderHelper { } 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 @@ public class ContentProviderHelper { 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 @@ public class ContentProviderHelper { 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 fb8b5d480b27..d8d8cccc05f3 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 @@ final class ContentProviderRecord implements ComponentName.WithComponentName { 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 @@ final class ContentProviderRecord implements ComponentName.WithComponentName { 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="); |