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

Commit 26ebeb6f authored by Thomas Stuart's avatar Thomas Stuart
Browse files

Make Connection.setExtras() thread-safe 

A race condition in Connection.setExtras() can lead to a
ConcurrentModificationException. Rapid, successive AUDIO_STATE_CHANGED
broadcasts can trigger concurrent calls to setExtras() on the same
Connection instance. The method was not thread-safe, allowing one
thread to modify the mPreviousExtraKeys set while another thread was
iterating over it.

This change resolves the issue by wrapping the logic within setExtras()
in a synchronized (mExtrasLock) block. This ensures that all read
and write operations on the shared mPreviousExtraKeys member are atomic,
preventing the race condition.

This is a low-risk bug fix and does not require a flag. The
risk of a deadlock is minimal because the lock is short-lived and does
not call out to external listeners while being held, which avoids
potential re-entrancy issues.

Flag: EXEMPT thread saftey fix (low regression risk)
Fixes: 438071847
Test: none
Change-Id: I36754e11b2cc460aefd0509b220a66541753edb6
parent a4f3d47a

telecomm/framework/java/android/telecom/Connection.java

+24 −23
Original line number Diff line number Diff line
@@ -3118,12 +3118,12 @@ public abstract class Connection extends Conferenceable { 
     */

    public final void setExtras(@Nullable Bundle extras) {

        checkImmutable();



        synchronized (mExtrasLock) {

            // Add/replace any new or changed extras values.

            putExtras(extras);



        // If we have used "setExtras" in the past, compare the key set from the last invocation to

        // the current one and remove any keys that went away.

            // If we have used "setExtras" in the past, compare the key set from the last invocation

            // to the current one and remove any keys that went away.

            if (mPreviousExtraKeys != null) {

                List<String> toRemove = new ArrayList<String>();

                for (String oldKey : mPreviousExtraKeys) {
@@ -3136,8 +3136,8 @@ public abstract class Connection extends Conferenceable { 
                }

            }



        // Track the keys the last time set called setExtras.  This way, the next time setExtras is

        // called we can see if the caller has removed any extras values.

            // Track the keys the last time set called setExtras.  This way, the next time setExtras

            // is called we can see if the caller has removed any extras values.

            if (mPreviousExtraKeys == null) {

                mPreviousExtraKeys = new ArraySet<String>();

            }
@@ -3146,6 +3146,7 @@ public abstract class Connection extends Conferenceable { 
                mPreviousExtraKeys.addAll(extras.keySet());

            }

        }

    }



    /**

     * Adds some extras to this {@code Connection}.  Existing keys are replaced and new ones are