From 55e15bcc3a3c7cba41f1e1e9eabf6fe316838018 Mon Sep 17 00:00:00 2001 From: Michal Karpinski Date: Thu, 2 Mar 2017 12:05:52 +0000 Subject: [PATCH] Use iterator to remove elements from collection when traversing it Per Java documentation "Iterator.remove is the only safe way to modify a collection during iteration; the behavior is unspecified if the underlying collection is modified in any other way while the iteration is in progress." Test: updated gmscore to check that onPackageRemoved() works correctly Bug: 35846264 Change-Id: I878e581db3bf60b9c6b2cf861f66d8b427202b02 --- .../server/backup/TransportManager.java | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/services/backup/java/com/android/server/backup/TransportManager.java b/services/backup/java/com/android/server/backup/TransportManager.java index b9dbe13af7c8c..619ddb16ee9b4 100644 --- a/services/backup/java/com/android/server/backup/TransportManager.java +++ b/services/backup/java/com/android/server/backup/TransportManager.java @@ -41,6 +41,7 @@ import com.android.internal.backup.IBackupTransport; import com.android.server.EventLogTags; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -103,13 +104,18 @@ class TransportManager { // been removed from mBoundTransports because onServiceDisconnected would already been // called on TransportConnection objects. synchronized (mTransportLock) { - for (ComponentName transport : mValidTransports.keySet()) { - if (transport.getPackageName().equals(packageName)) { - TransportConnection removed = mValidTransports.remove(transport); - if (removed != null) { - mContext.unbindService(removed); - log_verbose("Package removed, Removing transport: " + - transport.flattenToShortString()); + Iterator> iter = + mValidTransports.entrySet().iterator(); + while (iter.hasNext()) { + Map.Entry validTransport = iter.next(); + ComponentName componentName = validTransport.getKey(); + if (componentName.getPackageName().equals(packageName)) { + TransportConnection transportConnection = validTransport.getValue(); + iter.remove(); + if (transportConnection != null) { + mContext.unbindService(transportConnection); + log_verbose("Package removed, removing transport: " + + componentName.flattenToShortString()); } } } @@ -278,15 +284,16 @@ class TransportManager { } private void tryBindTransport(ServiceInfo transport) { - Slog.d(TAG, "Binding to transport: " + transport.getComponentName().flattenToShortString()); + final ComponentName transportComponentName = transport.getComponentName(); + Slog.d(TAG, "Binding to transport: " + transportComponentName.flattenToShortString()); // TODO: b/22388012 (Multi user backup and restore) - TransportConnection connection = new TransportConnection(transport.getComponentName()); - if (bindToTransport(transport.getComponentName(), connection)) { + TransportConnection connection = new TransportConnection(transportComponentName); + if (bindToTransport(transportComponentName, connection)) { synchronized (mTransportLock) { - mValidTransports.put(transport.getComponentName(), connection); + mValidTransports.put(transportComponentName, connection); } } else { - Slog.w(TAG, "Couldn't bind to transport " + transport.getComponentName()); + Slog.w(TAG, "Couldn't bind to transport " + transportComponentName); } }