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

Commit b30a1b4e authored by Abdelrahman Daim's avatar Abdelrahman Daim
Browse files

Fix race condition in process observer 



Summary: There is a race condition in dispatching the changes to the process
observer. When a ProcessChangeItem is initialized and retrieved during
enqueueProcessChangeItemLocked(), the change information will be set
with the handle retrieved. The change item will be processed in the UI
thread via a message.
However, there could be race condition. When the change item is
processed in the UI thread, its actual change information is not yet
populated and hence the dispatch will find no changes and do nothing.
This diff makes creating the ProcessChangeItem to be atomic so that
when the message to process is sent, it will have all the information.

Test: Successful build on master

Signed-off-by: default avatarAbdelrahman Daim <adaim@meta.com>

Change-Id: I5e0e5992c41dfd59c23bacdefcd8a294b13fc6be
parent f824197c

services/core/java/com/android/server/am/ActivityManagerService.java

+2 −4
Original line number Diff line number Diff line
@@ -17400,10 +17400,8 @@ public class ActivityManagerService extends IActivityManager.Stub 
            }















            psr.setReportedForegroundServiceTypes(fgServiceTypes);













            ProcessChangeItem item = mProcessList.enqueueProcessChangeItemLocked(













                    proc.getPid(), proc.info.uid);













            item.changes |= ProcessChangeItem.CHANGE_FOREGROUND_SERVICES;













            item.foregroundServiceTypes = fgServiceTypes;













            mProcessList.enqueueProcessChangeItemLocked(proc.getPid(), proc.info.uid,













                    ProcessChangeItem.CHANGE_FOREGROUND_SERVICES, fgServiceTypes);















        }















        if (oomAdj) {















            updateOomAdjLocked(proc, OOM_ADJ_REASON_UI_VISIBILITY);

























services/core/java/com/android/server/am/OomAdjuster.java



+5
−7




























Original line number
Diff line number
Diff line








@@ -3588,14 +3588,12 @@ public class OomAdjuster {













        if (changes != 0) {















            if (DEBUG_PROCESS_OBSERVERS) Slog.i(TAG_PROCESS_OBSERVERS,















                    "Changes in " + app + ": " + changes);













            ActivityManagerService.ProcessChangeItem item =













                    mProcessList.enqueueProcessChangeItemLocked(app.getPid(), app.info.uid);













            item.changes |= changes;













            item.foregroundActivities = state.hasRepForegroundActivities();













            mProcessList.enqueueProcessChangeItemLocked(app.getPid(), app.info.uid,













                    changes, state.hasRepForegroundActivities());















            if (DEBUG_PROCESS_OBSERVERS) Slog.i(TAG_PROCESS_OBSERVERS,













                    "Item " + Integer.toHexString(System.identityHashCode(item))













                            + " " + app.toShortString() + ": changes=" + item.changes













                            + " foreground=" + item.foregroundActivities













                    "Enqueued process change item for "













                            + app.toShortString() + ": changes=" + changes













                            + " foreground=" + state.hasRepForegroundActivities()















                            + " type=" + state.getAdjType() + " source=" + state.getAdjSource()















                            + " target=" + state.getAdjTarget());















        }

































services/core/java/com/android/server/am/ProcessList.java



+53
−36




























Original line number
Diff line number
Diff line








@@ -4987,8 +4987,26 @@ public final class ProcessList {













    }































    @GuardedBy("mService")













    ProcessChangeItem enqueueProcessChangeItemLocked(int pid, int uid) {













    void enqueueProcessChangeItemLocked(int pid, int uid, int changes, int foregroundServicetypes) {















        synchronized (mProcessChangeLock) {













            final ProcessChangeItem item = enqueueProcessChangeItemLocked(pid, uid);













            item.changes |= changes;













            item.foregroundServiceTypes = foregroundServicetypes;













        }













    }



























    @GuardedBy("mService")













    void enqueueProcessChangeItemLocked(int pid, int uid, int changes,













            boolean hasForegroundActivities) {













        synchronized (mProcessChangeLock) {













            final ProcessChangeItem item = enqueueProcessChangeItemLocked(pid, uid);













            item.changes |= changes;













            item.foregroundActivities = hasForegroundActivities;













        }













    }



























    @GuardedBy({"mService", "mProcessChangeLock"})













    private ProcessChangeItem enqueueProcessChangeItemLocked(int pid, int uid) {















        int i = mPendingProcessChanges.size() - 1;















        ActivityManagerService.ProcessChangeItem item = null;















        while (i >= 0) {










@@ -5031,7 +5049,6 @@ public final class ProcessList {





























        return item;















    }













    }































    @GuardedBy("mService")















    void scheduleDispatchProcessDiedLocked(int pid, int uid) {

































services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java



+4
−2




























Original line number
Diff line number
Diff line








@@ -224,8 +224,10 @@ public class MockingOomAdjusterTests {













        doCallRealMethod().when(mService).enqueueOomAdjTargetLocked(any(ProcessRecord.class));















        doCallRealMethod().when(mService).updateOomAdjPendingTargetsLocked(OOM_ADJ_REASON_ACTIVITY);















        setFieldValue(AppProfiler.class, profiler, "mProfilerLock", new Object());













        doReturn(new ActivityManagerService.ProcessChangeItem()).when(pr)













                .enqueueProcessChangeItemLocked(anyInt(), anyInt());













        doNothing().when(pr).enqueueProcessChangeItemLocked(anyInt(), anyInt(), anyInt(),













                anyInt());













        doNothing().when(pr).enqueueProcessChangeItemLocked(anyInt(), anyInt(), anyInt(),













                anyBoolean());















        mService.mOomAdjuster = mService.mConstants.ENABLE_NEW_OOMADJ















                ? new OomAdjusterModernImpl(mService, mService.mProcessList,















                        new ActiveUids(mService, false), mInjector)