Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0c3154d3 authored by Dianne Hackborn's avatar Dianne Hackborn
Browse files

Fix issue #2163654: deadlock, runtime restart

Don't hold a lock when the activity thread is telling the activity manager
to release a provider.

This requires that the activity manager now keep a reference count on the
providers, because without the lock it is possible for activity thread to
call in to request the provider again before it has finished telling
about the release.

Change-Id: I5f912903891f4edae85e28819d4e6f14b8f2e688
parent 4625758d
Loading
Loading
Loading
Loading
+25 −14
Original line number Diff line number Diff line
@@ -4075,6 +4075,9 @@ public final class ActivityThread {
                if(prc.count == 0) {
                    // Schedule the actual remove asynchronously, since we
                    // don't know the context this will be called in.
                    // TODO: it would be nice to post a delayed message, so
                    // if we come back and need the same provider quickly
                    // we will still have it available.
                    Message msg = mH.obtainMessage(H.REMOVE_PROVIDER, provider);
                    mH.sendMessage(msg);
                } //end if
@@ -4085,22 +4088,35 @@ public final class ActivityThread {

    final void completeRemoveProvider(IContentProvider provider) {
        IBinder jBinder = provider.asBinder();
        String name = null;
        synchronized(mProviderMap) {
            ProviderRefCount prc = mProviderRefCountMap.get(jBinder);
            if(prc != null && prc.count == 0) {
                mProviderRefCountMap.remove(jBinder);
                //invoke removeProvider to dereference provider
                removeProviderLocked(provider);
                name = removeProviderLocked(provider);
            }
        }
        
        if (name != null) {
            try {
                if(localLOGV) Log.v(TAG, "removeProvider::Invoking " +
                        "ActivityManagerNative.removeContentProvider(" + name);
                ActivityManagerNative.getDefault().removeContentProvider(
                        getApplicationThread(), name);
            } catch (RemoteException e) {
                //do nothing content provider object is dead any way
            } //end catch
        }
    }
    
    public final void removeProviderLocked(IContentProvider provider) {
    public final String removeProviderLocked(IContentProvider provider) {
        if (provider == null) {
            return;
            return null;
        }
        IBinder providerBinder = provider.asBinder();
        boolean amRemoveFlag = false;

        String name = null;
        
        // remove the provider from mProviderMap
        Iterator<ProviderRecord> iter = mProviderMap.values().iterator();
@@ -4111,7 +4127,7 @@ public final class ActivityThread {
                //find if its published by this process itself
                if(pr.mLocalProvider != null) {
                    if(localLOGV) Log.i(TAG, "removeProvider::found local provider returning");
                    return;
                    return name;
                }
                if(localLOGV) Log.v(TAG, "removeProvider::Not local provider Unlinking " +
                        "death recipient");
@@ -4119,18 +4135,13 @@ public final class ActivityThread {
                myBinder.unlinkToDeath(pr, 0);
                iter.remove();
                //invoke remove only once for the very first name seen
                if(!amRemoveFlag) {
                    try {
                        if(localLOGV) Log.v(TAG, "removeProvider::Invoking " +
                                "ActivityManagerNative.removeContentProvider("+pr.mName);
                        ActivityManagerNative.getDefault().removeContentProvider(getApplicationThread(), pr.mName);
                        amRemoveFlag = true;
                    } catch (RemoteException e) {
                        //do nothing content provider object is dead any way
                    } //end catch
                if(name == null) {
                    name = pr.mName;
                }
            } //end if myBinder
        }  //end while iter
        
        return name;
    }

    final void removeDeadProvider(String name, IContentProvider provider) {
+22 −7
Original line number Diff line number Diff line
@@ -7543,7 +7543,12 @@ public final class ActivityManagerService extends ActivityManagerNative implemen
                            "Adding provider requested by "
                            + r.processName + " from process "
                            + cpr.info.processName);
                    r.conProviders.add(cpr);
                    Integer cnt = r.conProviders.get(cpr);
                    if (cnt == null) {
                        r.conProviders.put(cpr, new Integer(1));
                    } else {
                        r.conProviders.put(cpr, new Integer(cnt.intValue()+1));
                    }
                    cpr.clients.add(r);
                } else {
                    cpr.externals++;
@@ -7649,7 +7654,12 @@ public final class ActivityManagerService extends ActivityManagerNative implemen
                            "Adding provider requested by "
                            + r.processName + " from process "
                            + cpr.info.processName);
                    r.conProviders.add(cpr);
                    Integer cnt = r.conProviders.get(cpr);
                    if (cnt == null) {
                        r.conProviders.put(cpr, new Integer(1));
                    } else {
                        r.conProviders.put(cpr, new Integer(cnt.intValue()+1));
                    }
                    cpr.clients.add(r);
                } else {
                    cpr.externals++;
@@ -7726,8 +7736,13 @@ public final class ActivityManagerService extends ActivityManagerNative implemen
                        + cpr.info.name + " in process " + r.processName);
                return;
            } else {
                Integer cnt = r.conProviders.get(localCpr);
                if (cnt == null || cnt.intValue() <= 1) {
                    localCpr.clients.remove(r);
                    r.conProviders.remove(localCpr);
                } else {
                    r.conProviders.put(localCpr, new Integer(cnt.intValue()-1));
                }
            }
            updateOomAdjLocked();
        }
@@ -9914,7 +9929,7 @@ public final class ActivityManagerService extends ActivityManagerNative implemen
        // Unregister from connected content providers.
        if (!app.conProviders.isEmpty()) {
            Iterator it = app.conProviders.iterator();
            Iterator it = app.conProviders.keySet().iterator();
            while (it.hasNext()) {
                ContentProviderRecord cpr = (ContentProviderRecord)it.next();
                cpr.clients.remove(app);
+2 −1
Original line number Diff line number Diff line
@@ -94,7 +94,8 @@ class ProcessRecord implements Watchdog.PssRequestor {
    // class (String) -> ContentProviderRecord
    final HashMap pubProviders = new HashMap(); 
    // All ContentProviderRecord process is using
    final HashSet conProviders = new HashSet(); 
    final HashMap<ContentProviderRecord, Integer> conProviders
            = new HashMap<ContentProviderRecord, Integer>(); 
    
    boolean persistent;         // always keep this application running?
    boolean crashing;           // are we in the process of crashing?