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

Commit bd698f15 authored by Fabián Kozynski's avatar Fabián Kozynski
Browse files

Prevent ConcurrentModificationException

While dispatching a change to callbacks, if one of them removes itself
as part of the operation, it would cause a
ConcurrentModificationException (because it's in the same thread, so
it's re-entrant).

Prevent this by copying the list before dispatching.

Test: atest CastControllerImplTest
Fixes: 317700495
Flag: None

Change-Id: I8fed9957dd167437f3c3e9bde3ccbb48b0b4f6c3
parent 77c5fef8
Loading
Loading
Loading
Loading
+5 −0
Original line number Diff line number Diff line
@@ -21,6 +21,11 @@ import androidx.lifecycle.Lifecycle.Event;
import androidx.lifecycle.LifecycleEventObserver;
import androidx.lifecycle.LifecycleOwner;

/**
 * Implementation of the collection used and thread guarantees are left to the discretion of the
 * client. Consider using {@link com.android.systemui.util.ListenerSet} to prevent concurrent
 * modification exceptions.
 */
public interface CallbackController<T> {

    /** Add a callback */
+8 −3
Original line number Diff line number Diff line
@@ -67,9 +67,7 @@ public class ReduceBrightColorsController implements
                synchronized (mListeners) {
                    if (setting != null && mListeners.size() != 0) {
                        if (setting.equals(Settings.Secure.REDUCE_BRIGHT_COLORS_ACTIVATED)) {
                            for (Listener listener : mListeners) {
                                listener.onActivated(mManager.isReduceBrightColorsActivated());
                            }
                            dispatchOnActivated(mManager.isReduceBrightColorsActivated());
                        }
                    }
                }
@@ -125,6 +123,13 @@ public class ReduceBrightColorsController implements
        mManager.setReduceBrightColorsActivated(activated);
    }

    private void dispatchOnActivated(boolean activated) {
        ArrayList<Listener> copy = new ArrayList<>(mListeners);
        for (Listener l : copy) {
            l.onActivated(activated);
        }
    }

    /**
     * Listener invoked whenever the Reduce Bright Colors settings are changed.
     */
+6 −3
Original line number Diff line number Diff line
@@ -104,7 +104,8 @@ public class ManagedProfileControllerImpl implements ManagedProfileController {
    }

    private void notifyManagedProfileRemoved() {
        for (Callback callback : mCallbacks) {
        ArrayList<Callback> copy = new ArrayList<>(mCallbacks);
        for (Callback callback : copy) {
            callback.onManagedProfileRemoved();
        }
    }
@@ -148,7 +149,8 @@ public class ManagedProfileControllerImpl implements ManagedProfileController {
        @Override
        public void onUserChanged(int newUser, @NonNull Context userContext) {
            reloadManagedProfiles();
            for (Callback callback : mCallbacks) {
            ArrayList<Callback> copy = new ArrayList<>(mCallbacks);
            for (Callback callback : copy) {
                callback.onManagedProfileChanged();
            }
        }
@@ -156,7 +158,8 @@ public class ManagedProfileControllerImpl implements ManagedProfileController {
        @Override
        public void onProfilesChanged(@NonNull List<UserInfo> profiles) {
            reloadManagedProfiles();
            for (Callback callback : mCallbacks) {
            ArrayList<Callback> copy = new ArrayList<>(mCallbacks);
            for (Callback callback : copy) {
                callback.onManagedProfileChanged();
            }
        }
+20 −30
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import static android.os.BatteryManager.CHARGING_POLICY_ADAPTIVE_LONGLIFE;
import static android.os.BatteryManager.CHARGING_POLICY_DEFAULT;
import static android.os.BatteryManager.EXTRA_CHARGING_STATUS;
import static android.os.BatteryManager.EXTRA_PRESENT;

import static com.android.settingslib.fuelgauge.BatterySaverLogging.SAVER_ENABLED_QS;
import static com.android.systemui.util.DumpUtilsKt.asIndenting;

@@ -61,6 +62,7 @@ import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;

import javax.annotation.concurrent.GuardedBy;

@@ -448,50 +450,38 @@ public class BatteryControllerImpl extends BroadcastReceiver implements BatteryC
        firePowerSaveChanged();
    }

    protected void fireBatteryLevelChanged() {
        mLogger.logBatteryLevelChangedCallback(mLevel, mPluggedIn, mCharging);
    protected final void dispatchSafeChange(Consumer<BatteryStateChangeCallback> action) {
        ArrayList<BatteryStateChangeCallback> copy;
        synchronized (mChangeCallbacks) {
            final int N = mChangeCallbacks.size();
            for (int i = 0; i < N; i++) {
                mChangeCallbacks.get(i).onBatteryLevelChanged(mLevel, mPluggedIn, mCharging);
            copy = new ArrayList<>(mChangeCallbacks);
        }
        final int n = copy.size();
        for (int i = 0; i < n; i++) {
            action.accept(copy.get(i));
        }
    }

    private void fireBatteryUnknownStateChanged() {
        synchronized (mChangeCallbacks) {
            final int n = mChangeCallbacks.size();
            for (int i = 0; i < n; i++) {
                mChangeCallbacks.get(i).onBatteryUnknownStateChanged(mStateUnknown);
            }
    protected void fireBatteryLevelChanged() {
        mLogger.logBatteryLevelChangedCallback(mLevel, mPluggedIn, mCharging);
        dispatchSafeChange(
                (callback) -> callback.onBatteryLevelChanged(mLevel, mPluggedIn, mCharging));
    }

    private void fireBatteryUnknownStateChanged() {
        dispatchSafeChange((callback) -> callback.onBatteryUnknownStateChanged(mStateUnknown));
    }

    private void firePowerSaveChanged() {
        synchronized (mChangeCallbacks) {
            final int N = mChangeCallbacks.size();
            for (int i = 0; i < N; i++) {
                mChangeCallbacks.get(i).onPowerSaveChanged(mPowerSave);
            }
        }
        dispatchSafeChange((callback) -> callback.onPowerSaveChanged(mPowerSave));
    }

    private void fireIsBatteryDefenderChanged() {
        synchronized (mChangeCallbacks) {
            final int n = mChangeCallbacks.size();
            for (int i = 0; i < n; i++) {
                mChangeCallbacks.get(i).onIsBatteryDefenderChanged(mIsBatteryDefender);
            }
        }
        dispatchSafeChange((callback) -> callback.onIsBatteryDefenderChanged(mIsBatteryDefender));
    }

    private void fireIsIncompatibleChargingChanged() {
        synchronized (mChangeCallbacks) {
            final int n = mChangeCallbacks.size();
            for (int i = 0; i < n; i++) {
                mChangeCallbacks.get(i).onIsIncompatibleChargingChanged(mIsIncompatibleCharging);
            }
        }
        dispatchSafeChange(
                (callback) -> callback.onIsIncompatibleChargingChanged(mIsIncompatibleCharging));
    }

    @Override
+2 −0
Original line number Diff line number Diff line
@@ -436,6 +436,8 @@ public class BluetoothControllerImpl implements BluetoothController, BluetoothCa
    @Override
    public void onServiceDisconnected() {}

    // IMPORTANT: This handler guarantees that any operations on the list of callbacks is
    // sequential, so no concurrent exceptions
    private final class H extends Handler {
        private final ArrayList<BluetoothController.Callback> mCallbacks = new ArrayList<>();

Loading