Skip to content

Commit 6ae8d18

Browse files
author
Dianne Hackborn
committed
Fix (mostly) issue #5109947: Race condition between retrieving a...
...content provider and updating its oom adj This introduces the concept of an "unstable" reference on a content provider. When holding such a reference (and no normal stable ref), the content provider dying will not cause the client process to be killed. This is used in ContentResolver.query(), .openAssetFileDescriptor(), and .openTypedAssetFileDescriptor() to first access the provider with an unstable reference, and if at the point of calling into the provider we find it is dead then acquiring a new stable reference and doing the operation again. Thus if the provider process dies at any point until we get the result back, our own process will not be killed and we can safely retry the operation. Arguably there is still the potential for a race -- if somehow the provider is killed way late by the OOM killer after the query or open has returned -- but this should now be *extremely* unlikely. We also continue to have the issue with the other calls, but these are much less critical, and the same model can't be used there (we wouldn't want to execute two insert operations for example). The implementation of this required some significant changes to the underlying plumbing of content providers, now keeping track of the two different reference counts, and managing them appropriately. To facilitate this, the activity manager now has a formal connection object for a client reference on a content provider, which hands to the application when opening the provider. These changes have allowed a lot of the code to be cleaned up and subtle issues closed. For example, when a process is crashing, we now have a much better idea of the state of content provider clients (olding a stable ref, unstable ref, or waiting for it to launch), so that we can correctly handle each of these. The client side code is also a fair amount cleaner, though in the future there is more than should be done. In particular, the two ProviderClientRecord and ProviderRefCount classes should be combined into one, part of which is exposed to the ContentResolver internal API as a reference on a content provider with methods for updating reference counts and such. Some day we'll do that. Change-Id: I87b10d1b67573ab899e09ca428f1b556fd669c8c
1 parent 3dac022 commit 6ae8d18

File tree

16 files changed

+1178
-531
lines changed

16 files changed

+1178
-531
lines changed

core/java/android/app/ActivityManagerNative.java

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,8 @@ public boolean onTransact(int code, Parcel data, Parcel reply, int flags)
580580
IBinder b = data.readStrongBinder();
581581
IApplicationThread app = ApplicationThreadNative.asInterface(b);
582582
String name = data.readString();
583-
ContentProviderHolder cph = getContentProvider(app, name);
583+
boolean stable = data.readInt() != 0;
584+
ContentProviderHolder cph = getContentProvider(app, name, stable);
584585
reply.writeNoException();
585586
if (cph != null) {
586587
reply.writeInt(1);
@@ -617,12 +618,30 @@ public boolean onTransact(int code, Parcel data, Parcel reply, int flags)
617618
return true;
618619
}
619620

621+
case REF_CONTENT_PROVIDER_TRANSACTION: {
622+
data.enforceInterface(IActivityManager.descriptor);
623+
IBinder b = data.readStrongBinder();
624+
int stable = data.readInt();
625+
int unstable = data.readInt();
626+
boolean res = refContentProvider(b, stable, unstable);
627+
reply.writeNoException();
628+
reply.writeInt(res ? 1 : 0);
629+
return true;
630+
}
631+
632+
case UNSTABLE_PROVIDER_DIED_TRANSACTION: {
633+
data.enforceInterface(IActivityManager.descriptor);
634+
IBinder b = data.readStrongBinder();
635+
unstableProviderDied(b);
636+
reply.writeNoException();
637+
return true;
638+
}
639+
620640
case REMOVE_CONTENT_PROVIDER_TRANSACTION: {
621641
data.enforceInterface(IActivityManager.descriptor);
622642
IBinder b = data.readStrongBinder();
623-
IApplicationThread app = ApplicationThreadNative.asInterface(b);
624-
String name = data.readString();
625-
removeContentProvider(app, name);
643+
boolean stable = data.readInt() != 0;
644+
removeContentProvider(b, stable);
626645
reply.writeNoException();
627646
return true;
628647
}
@@ -2314,13 +2333,13 @@ public void reportThumbnail(IBinder token,
23142333
reply.recycle();
23152334
}
23162335
public ContentProviderHolder getContentProvider(IApplicationThread caller,
2317-
String name) throws RemoteException
2318-
{
2336+
String name, boolean stable) throws RemoteException {
23192337
Parcel data = Parcel.obtain();
23202338
Parcel reply = Parcel.obtain();
23212339
data.writeInterfaceToken(IActivityManager.descriptor);
23222340
data.writeStrongBinder(caller != null ? caller.asBinder() : null);
23232341
data.writeString(name);
2342+
data.writeInt(stable ? 1 : 0);
23242343
mRemote.transact(GET_CONTENT_PROVIDER_TRANSACTION, data, reply, 0);
23252344
reply.readException();
23262345
int res = reply.readInt();
@@ -2352,7 +2371,7 @@ public ContentProviderHolder getContentProviderExternal(String name, IBinder tok
23522371
return cph;
23532372
}
23542373
public void publishContentProviders(IApplicationThread caller,
2355-
List<ContentProviderHolder> providers) throws RemoteException
2374+
List<ContentProviderHolder> providers) throws RemoteException
23562375
{
23572376
Parcel data = Parcel.obtain();
23582377
Parcel reply = Parcel.obtain();
@@ -2364,14 +2383,38 @@ public void publishContentProviders(IApplicationThread caller,
23642383
data.recycle();
23652384
reply.recycle();
23662385
}
2367-
2368-
public void removeContentProvider(IApplicationThread caller,
2369-
String name) throws RemoteException {
2386+
public boolean refContentProvider(IBinder connection, int stable, int unstable)
2387+
throws RemoteException {
23702388
Parcel data = Parcel.obtain();
23712389
Parcel reply = Parcel.obtain();
23722390
data.writeInterfaceToken(IActivityManager.descriptor);
2373-
data.writeStrongBinder(caller != null ? caller.asBinder() : null);
2374-
data.writeString(name);
2391+
data.writeStrongBinder(connection);
2392+
data.writeInt(stable);
2393+
data.writeInt(unstable);
2394+
mRemote.transact(REF_CONTENT_PROVIDER_TRANSACTION, data, reply, 0);
2395+
reply.readException();
2396+
boolean res = reply.readInt() != 0;
2397+
data.recycle();
2398+
reply.recycle();
2399+
return res;
2400+
}
2401+
public void unstableProviderDied(IBinder connection) throws RemoteException {
2402+
Parcel data = Parcel.obtain();
2403+
Parcel reply = Parcel.obtain();
2404+
data.writeInterfaceToken(IActivityManager.descriptor);
2405+
data.writeStrongBinder(connection);
2406+
mRemote.transact(UNSTABLE_PROVIDER_DIED_TRANSACTION, data, reply, 0);
2407+
reply.readException();
2408+
data.recycle();
2409+
reply.recycle();
2410+
}
2411+
2412+
public void removeContentProvider(IBinder connection, boolean stable) throws RemoteException {
2413+
Parcel data = Parcel.obtain();
2414+
Parcel reply = Parcel.obtain();
2415+
data.writeInterfaceToken(IActivityManager.descriptor);
2416+
data.writeStrongBinder(connection);
2417+
data.writeInt(stable ? 1 : 0);
23752418
mRemote.transact(REMOVE_CONTENT_PROVIDER_TRANSACTION, data, reply, 0);
23762419
reply.readException();
23772420
data.recycle();

0 commit comments

Comments
 (0)