There is a small gap in the SynchronizedWeakHashSet implementation. T… (#24015)

Summary:
There is a small gap in the SynchronizedWeakHashSet implementation - the containsKey method of the WeakHashMap is modifying hence calling it during the iteration might cause ConcurrentModificationException. Added a command DO_IF_CONTAINS to safely handle this case.

[Android] [Bugfix] - Should fix a ConcurrentModificationException in onResume.
Pull Request resolved: #24015

Reviewed By: mdvacca

Differential Revision: D14507342

Pulled By: fkgozali

fbshipit-source-id: 2998bffb06e2cbacd8df1780964355842b1cc4a0

Author: dryganets

ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java

@@ -176,14 +176,17 @@ public void addLifecycleEventListener(final LifecycleEventListener listener) {
               new Runnable() {
                 @Override
                 public void run() {
-                  if (!mLifecycleEventListeners.contains(listener)) {
-                    return;
-                  }
-                  try {
-                    listener.onHostResume();
-                  } catch (RuntimeException e) {
-                    handleException(e);
-                  }
+
+                  mLifecycleEventListeners.doIfContains(listener, new Runnable() {
+                    @Override
+                    public void run() {
+                      try {
+                        listener.onHostResume();
+                      } catch (RuntimeException e) {
+                        handleException(e);
+                      }
+                    }
+                  });
                 }
               });
           break;

ReactAndroid/src/main/java/com/facebook/react/bridge/SynchronizedWeakHashSet.java

@@ -24,9 +24,15 @@ public class SynchronizedWeakHashSet<T> {
   private Queue<Pair<T, Command>> mPendingOperations = new ArrayDeque<>();
   private boolean mIterating;
 
-  public boolean contains(T item) {
+  public void doIfContains(T item, Runnable runnable) {
     synchronized (mMap) {
-      return mMap.containsKey(item);
+      if (mIterating) {
+        mPendingOperations.add(new Pair<>(item, Command.newDoIfContains(runnable)));
+      } else {
+        if (mMap.containsKey(item)) {
+          runnable.run();
+        }
+      }
     }
   }
 
@@ -61,13 +67,19 @@ public void iterate(Iteration<T> iterated) {
 
       while (!mPendingOperations.isEmpty()) {
         Pair<T, Command> pair = mPendingOperations.poll();
-        switch (pair.second) {
+        Command command = pair.second;
+        switch (command.getType()) {
           case ADD:
             mMap.put(pair.first, null);
             break;
           case REMOVE:
             mMap.remove(pair.first);
             break;
+          case DO_IF_CONTAINS:
+            if (mMap.containsKey(pair.first)) {
+              command.execute();
+            }
+            break;
             default:
               throw new AssertionException("Unsupported command" + pair.second);
         }
@@ -79,8 +91,40 @@ public interface Iteration<T> {
     void iterate(T item);
   }
 
-  private enum Command {
+  private enum CommandType {
     ADD,
-    REMOVE
+    REMOVE,
+    DO_IF_CONTAINS
+  }
+
+  private static class Command {
+    public static final Command ADD = new Command(CommandType.ADD);
+    public static final Command REMOVE = new Command(CommandType.REMOVE);
+
+    private CommandType mType;
+    private Runnable mRunnable;
+
+    public static Command newDoIfContains(Runnable runnable) {
+      return new Command(CommandType.DO_IF_CONTAINS, runnable);
+    }
+
+    private Command(CommandType type) {
+      this(type, null);
+    }
+
+    private Command(CommandType type, Runnable runnable) {
+      mType = type;
+      mRunnable = runnable;
+    }
+
+    public CommandType getType() {
+      return mType;
+    }
+
+    public void execute() {
+      if (mRunnable != null) {
+        mRunnable.run();
+      }
+    }
   }
 }