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

Commit 229e3820 authored by Jay Shrauner's avatar Jay Shrauner
Browse files

Prevent ConcurrentModificationExceptions 

Use sets backed by ConcurrentHashMaps instead of HashSets, and
CopyOnWriteArrayLists instead of ArrayLists, to prevent concurrent
exceptions if listeners try to remove themselves in callbacks while
iterating over the listeners.

Bug:16325026
Change-Id: I55e081eda6ba19fa466bbf019c648bbdaf833c33
parent d438deff

telecomm/java/android/telecomm/Call.java

+25 −32
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import java.util.Collections; 
import java.util.List;

import java.util.Map;

import java.util.Objects;

import java.util.concurrent.CopyOnWriteArrayList;



/**

 * Represents an ongoing phone call that the in-call app should present to the user.
@@ -364,7 +365,7 @@ public final class Call { 
    private final InCallAdapter mInCallAdapter;

    private final List<Call> mChildren = new ArrayList<>();

    private final List<Call> mUnmodifiableChildren = Collections.unmodifiableList(mChildren);

    private final List<Listener> mListeners = new ArrayList<>();

    private final List<Listener> mListeners = new CopyOnWriteArrayList<>();

    private final List<Call> mConferenceableCalls = new ArrayList<>();

    private final List<Call> mUnmodifiableConferenceableCalls =

            Collections.unmodifiableList(mConferenceableCalls);
@@ -589,8 +590,10 @@ public final class Call { 
     * @param listener A {@code Listener}.

     */

    public void removeListener(Listener listener) {

        if (listener != null) {

            mListeners.remove(listener);

        }

    }



    /** {@hide} */

    Call(Phone phone, String telecommCallId, InCallAdapter inCallAdapter) {
@@ -709,72 +712,62 @@ public final class Call { 
    }



    private void fireStateChanged(int newState) {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onStateChanged(this, newState);

        for (Listener listener : mListeners) {

            listener.onStateChanged(this, newState);

        }

    }



    private void fireParentChanged(Call newParent) {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onParentChanged(this, newParent);

        for (Listener listener : mListeners) {

            listener.onParentChanged(this, newParent);

        }

    }



    private void fireChildrenChanged(List<Call> children) {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onChildrenChanged(this, children);

        for (Listener listener : mListeners) {

            listener.onChildrenChanged(this, children);

        }

    }



    private void fireDetailsChanged(Details details) {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onDetailsChanged(this, details);

        for (Listener listener : mListeners) {

            listener.onDetailsChanged(this, details);

        }

    }



    private void fireCannedTextResponsesLoaded(List<String> cannedTextResponses) {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onCannedTextResponsesLoaded(this, cannedTextResponses);

        for (Listener listener : mListeners) {

            listener.onCannedTextResponsesLoaded(this, cannedTextResponses);

        }

    }



    private void fireVideoCallChanged(InCallService.VideoCall videoCall) {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onVideoCallChanged(this, videoCall);

        for (Listener listener : mListeners) {

            listener.onVideoCallChanged(this, videoCall);

        }

    }



    private void firePostDialWait(String remainingPostDialSequence) {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onPostDialWait(this, remainingPostDialSequence);

        for (Listener listener : mListeners) {

            listener.onPostDialWait(this, remainingPostDialSequence);

        }

    }



    private void fireStartActivity(PendingIntent intent) {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onStartActivity(this, intent);

        for (Listener listener : mListeners) {

            listener.onStartActivity(this, intent);

        }

    }



    private void fireCallDestroyed() {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onCallDestroyed(this);

        for (Listener listener : mListeners) {

            listener.onCallDestroyed(this);

        }

    }



    private void fireConferenceableCallsChanged() {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onConferenceableCallsChanged(this, mUnmodifiableConferenceableCalls);

        for (Listener listener : mListeners) {

            listener.onConferenceableCallsChanged(this, mUnmodifiableConferenceableCalls);

        }

    }

telecomm/java/android/telecomm/Connection.java

+13 −10
Original line number Diff line number Diff line
@@ -32,7 +32,7 @@ import java.util.ArrayList; 
import java.util.Collections;

import java.util.List;

import java.util.Set;

import java.util.concurrent.CopyOnWriteArraySet;

import java.util.concurrent.ConcurrentHashMap;



/**

 * Represents a connection to a remote endpoint that carries voice traffic.
@@ -448,7 +448,13 @@ public abstract class Connection { 
        }

    };



    private final Set<Listener> mListeners = new CopyOnWriteArraySet<>();

    /**

     * ConcurrentHashMap constructor params: 8 is initial table size, 0.9f is

     * load factor before resizing, 1 means we only expect a single thread to

     * access the map so make only a single shard

     */

    private final Set<Listener> mListeners = Collections.newSetFromMap(

            new ConcurrentHashMap<Listener, Boolean>(8, 0.9f, 1));

    private final List<Connection> mChildConnections = new ArrayList<>();

    private final List<Connection> mUnmodifiableChildConnections =

            Collections.unmodifiableList(mChildConnections);
@@ -587,7 +593,9 @@ public abstract class Connection { 
     * @hide

     */

    public final Connection removeConnectionListener(Listener l) {

        if (l != null) {

            mListeners.remove(l);

        }

        return this;

    }
@@ -874,15 +882,10 @@ public abstract class Connection { 
     * Tears down the Connection object.

     */

    public final void destroy() {

        // It is possible that onDestroy() will trigger the listener to remove itself which will

        // result in a concurrent modification exception. To counteract this we make a copy of the

        // listeners and iterate on that.

        for (Listener l : new ArrayList<>(mListeners)) {

            if (mListeners.contains(l)) {

        for (Listener l : mListeners) {

            l.onDestroyed(this);

        }

    }

    }



    /**

     * Requests that the framework use VOIP audio mode for this connection.

telecomm/java/android/telecomm/ConnectionServiceAdapter.java

+7 −2
Original line number Diff line number Diff line
@@ -36,8 +36,13 @@ import java.util.concurrent.ConcurrentHashMap; 
 * @hide

 */

final class ConnectionServiceAdapter implements DeathRecipient {

    /**

     * ConcurrentHashMap constructor params: 8 is initial table size, 0.9f is

     * load factor before resizing, 1 means we only expect a single thread to

     * access the map so make only a single shard

     */

    private final Set<IConnectionServiceAdapter> mAdapters = Collections.newSetFromMap(

            new ConcurrentHashMap<IConnectionServiceAdapter, Boolean>(2));

            new ConcurrentHashMap<IConnectionServiceAdapter, Boolean>(8, 0.9f, 1));



    ConnectionServiceAdapter() {

    }
@@ -53,7 +58,7 @@ final class ConnectionServiceAdapter implements DeathRecipient { 
    }



    void removeAdapter(IConnectionServiceAdapter adapter) {

        if (mAdapters.remove(adapter)) {

        if (adapter != null && mAdapters.remove(adapter)) {

            adapter.asBinder().unlinkToDeath(this, 0);

        }

    }

telecomm/java/android/telecomm/Phone.java

+13 −14
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import java.util.Collections; 
import java.util.List;

import java.util.Map;

import java.util.Objects;

import java.util.concurrent.CopyOnWriteArrayList;



/**

 * A unified virtual device providing a means of voice (and other) communication on a device.
@@ -89,7 +90,7 @@ public final class Phone { 


    private AudioState mAudioState;



    private final List<Listener> mListeners = new ArrayList<>();

    private final List<Listener> mListeners = new CopyOnWriteArrayList<>();



    /** {@hide} */

    Phone(InCallAdapter adapter) {
@@ -171,8 +172,10 @@ public final class Phone { 
     * @param listener A {@code Listener} object.

     */

    public final void removeListener(Listener listener) {

        if (listener != null) {

            mListeners.remove(listener);

        }

    }



    /**

     * Obtains the current list of {@code Call}s to be displayed by this in-call experience.
@@ -236,30 +239,26 @@ public final class Phone { 
    }



    private void fireCallAdded(Call call) {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onCallAdded(this, call);

        for (Listener listener : mListeners) {

            listener.onCallAdded(this, call);

        }

    }



    private void fireCallRemoved(Call call) {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onCallRemoved(this, call);

        for (Listener listener : mListeners) {

            listener.onCallRemoved(this, call);

        }

    }



    private void fireAudioStateChanged(AudioState audioState) {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onAudioStateChanged(this, audioState);

        for (Listener listener : mListeners) {

            listener.onAudioStateChanged(this, audioState);

        }

    }



    private void fireBringToForeground(boolean showDialpad) {

        Listener[] listeners = mListeners.toArray(new Listener[mListeners.size()]);

        for (int i = 0; i < listeners.length; i++) {

            listeners[i].onBringToForeground(this, showDialpad);

        for (Listener listener : mListeners) {

            listener.onBringToForeground(this, showDialpad);

        }

    }

telecomm/java/android/telecomm/RemoteConnection.java

+11 −5
Original line number Diff line number Diff line
@@ -184,8 +184,13 @@ public final class RemoteConnection { 


    private IConnectionService mConnectionService;

    private final String mConnectionId;

    /**

     * ConcurrentHashMap constructor params: 8 is initial table size, 0.9f is

     * load factor before resizing, 1 means we only expect a single thread to

     * access the map so make only a single shard

     */

    private final Set<Listener> mListeners = Collections.newSetFromMap(

            new ConcurrentHashMap<Listener, Boolean>(2));

            new ConcurrentHashMap<Listener, Boolean>(8, 0.9f, 1));

    private final Set<RemoteConnection> mConferenceableConnections = new HashSet<>();



    private int mState = Connection.STATE_NEW;
@@ -248,8 +253,10 @@ public final class RemoteConnection { 
     * @param listener A {@code Listener}.

     */

    public void removeListener(Listener listener) {

        if (listener != null) {

            mListeners.remove(listener);

        }

    }



    /**

     * Obtains the parent of this {@code RemoteConnection} in a conference, if any.
@@ -588,11 +595,10 @@ public final class RemoteConnection { 
                setDisconnected(DisconnectCause.ERROR_UNSPECIFIED, "Connection destroyed.");

            }



            Set<Listener> listeners = new HashSet<Listener>(mListeners);

            mListeners.clear();

            for (Listener l : listeners) {

            for (Listener l : mListeners) {

                l.onDestroyed(this);

            }

            mListeners.clear();



            mConnected = false;

        }