diff options
7 files changed, 160 insertions, 16 deletions
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index 7cec717f96e0..d67b98620f37 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -424,9 +424,14 @@ public final class ActivityThread extends ClientTransactionHandler { 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 @@ -440,7 +445,11 @@ public final class ActivityThread extends ClientTransactionHandler { @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; } } @@ -461,9 +470,8 @@ 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. - // 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>>(); @@ -1756,6 +1764,16 @@ 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.mLock) { + key.mHolder = holder; + key.mLock.notifyAll(); + } + } } private @NonNull SafeCancellationTransport createSafeCancellationTransport( @@ -6807,13 +6825,33 @@ 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); 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)) { @@ -6831,13 +6869,13 @@ public final class ActivityThread extends ClientTransactionHandler { 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 3d745831ce1c..e330a30de7b0 100644 --- a/core/java/android/app/ContentProviderHolder.java +++ b/core/java/android/app/ContentProviderHolder.java @@ -39,6 +39,11 @@ 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; @@ -59,6 +64,7 @@ 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 @@ -81,5 +87,6 @@ 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 24da50481df3..dc9918ade9c2 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 @@ 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 e0d702e98595..2bf9848304ad 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 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,5 +666,10 @@ 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 a838aadc404e..f250647d7711 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -1581,6 +1581,7 @@ 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; @@ -1917,6 +1918,11 @@ 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 5cc7aba736e1..bfba4afcd4e4 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.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; @@ -218,7 +219,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); + 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 @@ 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); + return cpr.newHolder(null, true); } if (ActivityManagerDebugConfig.DEBUG_PROVIDER) { @@ -513,6 +514,38 @@ 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; @@ -569,7 +602,7 @@ public class ContentProviderHelper { + " 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 @@ 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. @@ -635,6 +670,7 @@ 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); @@ -1504,6 +1540,9 @@ 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 d8d8cccc05f3..fb8b5d480b27 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 @@ final class ContentProviderRecord implements ComponentName.WithComponentName { 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 @@ 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="); |