From 48e2615128ff7ee674547f75ca561c1e405c2a8e Mon Sep 17 00:00:00 2001 From: Hall Liu Date: Wed, 7 Nov 2018 18:32:02 -0800 Subject: [PATCH] Fix flaky AnalytisTest by locking in EventRecord AnalyticsTests were flaking with concurrent modification exceptions. Fix this by locking mEvents properly in EventRecord. Bug: 119226554 Test: leave it to presubmit Change-Id: I4df0afe399fab148bbebcef51272c56f628ee18f --- .../android/telecom/Logging/EventManager.java | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/telecomm/java/android/telecom/Logging/EventManager.java b/telecomm/java/android/telecom/Logging/EventManager.java index 2bda6480b99e4..1342038c64775 100644 --- a/telecomm/java/android/telecom/Logging/EventManager.java +++ b/telecomm/java/android/telecom/Logging/EventManager.java @@ -180,7 +180,7 @@ public class EventManager { } } - private final List mEvents = new LinkedList<>(); + private final List mEvents = Collections.synchronizedList(new LinkedList<>()); private final Loggable mRecordEntry; public EventRecord(Loggable recordEntry) { @@ -197,7 +197,7 @@ public class EventManager { } public List getEvents() { - return mEvents; + return new LinkedList<>(mEvents); } public List extractEventTimings() { @@ -207,21 +207,24 @@ public class EventManager { LinkedList result = new LinkedList<>(); Map pendingResponses = new HashMap<>(); - for (Event event : mEvents) { - if (requestResponsePairs.containsKey(event.eventId)) { - // This event expects a response, so add that expected response to the maps - // of pending events. - for (EventManager.TimedEventPair p : requestResponsePairs.get(event.eventId)) { - pendingResponses.put(p.mResponse, new PendingResponse(event.eventId, - event.time, p.mTimeoutMillis, p.mName)); + synchronized (mEvents) { + for (Event event : mEvents) { + if (requestResponsePairs.containsKey(event.eventId)) { + // This event expects a response, so add that expected response to the maps + // of pending events. + for (EventManager.TimedEventPair p : requestResponsePairs.get( + event.eventId)) { + pendingResponses.put(p.mResponse, new PendingResponse(event.eventId, + event.time, p.mTimeoutMillis, p.mName)); + } } - } - PendingResponse pendingResponse = pendingResponses.remove(event.eventId); - if (pendingResponse != null) { - long elapsedTime = event.time - pendingResponse.requestEventTimeMillis; - if (elapsedTime < pendingResponse.timeoutMillis) { - result.add(new EventTiming(pendingResponse.name, elapsedTime)); + PendingResponse pendingResponse = pendingResponses.remove(event.eventId); + if (pendingResponse != null) { + long elapsedTime = event.time - pendingResponse.requestEventTimeMillis; + if (elapsedTime < pendingResponse.timeoutMillis) { + result.add(new EventTiming(pendingResponse.name, elapsedTime)); + } } } } @@ -233,7 +236,8 @@ public class EventManager { pw.print(mRecordEntry.getDescription()); pw.increaseIndent(); - for (Event event : mEvents) { + // Iterate over copy of events so that this doesn't hold the lock for too long. + for (Event event : getEvents()) { pw.print(event.timestampString); pw.print(" - "); pw.print(event.eventId);