From 37dfb429924be4373cc458816fae6d7eec0aa14d Mon Sep 17 00:00:00 2001 From: John Reck Date: Wed, 22 Feb 2017 17:01:30 -0800 Subject: [PATCH] Fix concurrent modification crash in onAlarm Change-Id: Idfd094f3c9ea59356440d6851ccd5abda36ca6ba Fixes: 35640585 Test: manual; after boot, opened the power menu to force system_server to spin up a ThreadedRenderer instance (and thus register itself as a callback on GraphicsStatsService). Then manually set the date forward by a day to trigger onAlarm and verified the system didn't reboot/crash. A systrace capture verified that the alarm fired and package:android (system_server) had a log rotation event. --- .../android/server/GraphicsStatsService.java | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/GraphicsStatsService.java b/services/core/java/com/android/server/GraphicsStatsService.java index bdd80e382867d..14a9a313ffeec 100644 --- a/services/core/java/com/android/server/GraphicsStatsService.java +++ b/services/core/java/com/android/server/GraphicsStatsService.java @@ -137,16 +137,21 @@ public class GraphicsStatsService extends IGraphicsStats.Stub { } private void onAlarm() { + // We need to make a copy since some of the callbacks won't be proxy and thus + // can result in a re-entrant acquisition of mLock that would result in a modification + // of mActive during iteration. + ActiveBuffer[] activeCopy; synchronized (mLock) { mRotateIsScheduled = false; scheduleRotateLocked(); - for (ActiveBuffer active : mActive) { - try { - active.mCallback.onRotateGraphicsStatsBuffer(); - } catch (RemoteException e) { - Log.w(TAG, String.format("Failed to notify '%s' (pid=%d) to rotate buffers", - active.mInfo.packageName, active.mPid), e); - } + activeCopy = mActive.toArray(new ActiveBuffer[0]); + } + for (ActiveBuffer active : activeCopy) { + try { + active.mCallback.onRotateGraphicsStatsBuffer(); + } catch (RemoteException e) { + Log.w(TAG, String.format("Failed to notify '%s' (pid=%d) to rotate buffers", + active.mInfo.packageName, active.mPid), e); } } // Give a few seconds for everyone to rotate before doing the cleanup