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

Commit 84fd7978 authored by Kevin Chyn's avatar Kevin Chyn Committed by Android Build Coastguard Worker
Browse files

Do not use Map.Entry outside of lock 

Map.Entry returned from Map#entrySet are tied to the backing map.
A change to the map is reflected to the Map.Entry. This causes
undefined behavior (including NPE) if we try to use them outside of
the lock. If a client removes their callback after we exit the lock
but before we invoke the client callbacks, we run into NPE. Instead,
lets copy the callback and executor into a separate data structure.

Fixes: 286512746
Test: atest DeviceStateControllerTests
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:ffb63d96f3a819ba86fbe543a203bf9124a18ad1)
Merged-In: I8ed29cf6e27f5c8020f120c3968896f627573a35
Change-Id: I8ed29cf6e27f5c8020f120c3968896f627573a35
parent 8cef0013

services/core/java/com/android/server/wm/DeviceStateController.java

+8 −5
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import android.annotation.CallbackExecutor; 
import android.annotation.NonNull;

import android.content.Context;

import android.util.ArrayMap;

import android.util.Pair;



import com.android.internal.R;

import com.android.internal.annotations.GuardedBy;
@@ -165,18 +166,20 @@ final class DeviceStateController { 


            // Make a copy here because it's possible that the consumer tries to remove a callback

            // while we're still iterating through the list, which would end up in a

            // ConcurrentModificationException.

            final List<Map.Entry<Consumer<DeviceState>, Executor>> entries = new ArrayList<>();

            // ConcurrentModificationException. Note that cannot use a List<Map.Entry> because the

            // entries are tied to the backing map. So, if a client removes a callback while

            // we are notifying clients, we will get a NPE.

            final List<Pair<Consumer<DeviceState>, Executor>> entries = new ArrayList<>();

            synchronized (mWmLock) {

                for (Map.Entry<Consumer<DeviceState>, Executor> entry

                        : mDeviceStateCallbacks.entrySet()) {

                    entries.add(entry);

                    entries.add(new Pair<>(entry.getKey(), entry.getValue()));

                }

            }



            for (int i = 0; i < entries.size(); i++) {

                Map.Entry<Consumer<DeviceState>, Executor> entry = entries.get(i);

                entry.getValue().execute(() -> entry.getKey().accept(mCurrentDeviceState));

                final Pair<Consumer<DeviceState>, Executor> entry = entries.get(i);

                entry.second.execute(() -> entry.first.accept(deviceState));

            }

        }

    }