Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This change fixes currently broken ReactContext listeners mechanism. #22318

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import com.facebook.react.bridge.queue.ReactQueueConfiguration;
import com.facebook.react.common.LifecycleState;
import java.lang.ref.WeakReference;
import java.util.concurrent.CopyOnWriteArraySet;
import javax.annotation.Nullable;

/**
Expand All @@ -32,10 +31,35 @@ public class ReactContext extends ContextWrapper {
"ReactContext#getJSModule should only happen once initialize() has been called on your " +
"native module.";

private final CopyOnWriteArraySet<LifecycleEventListener> mLifecycleEventListeners =
new CopyOnWriteArraySet<>();
private final CopyOnWriteArraySet<ActivityEventListener> mActivityEventListeners =
new CopyOnWriteArraySet<>();
private final SynchronizedWeakHashSet<LifecycleEventListener> mLifecycleEventListeners =
new SynchronizedWeakHashSet<>();
private final SynchronizedWeakHashSet<ActivityEventListener> mActivityEventListeners =
new SynchronizedWeakHashSet<>();


private final GuardedIteration<LifecycleEventListener> mResumeIteration =
new GuardedIteration<LifecycleEventListener>() {
@Override
public void onIterate(LifecycleEventListener listener) {
listener.onHostResume();
}
};

private final GuardedIteration<LifecycleEventListener> mPauseIteration =
new GuardedIteration<LifecycleEventListener>() {
@Override
public void onIterate(LifecycleEventListener listener) {
listener.onHostPause();
}
};

private final GuardedIteration<LifecycleEventListener> mDestroyIteration =
new GuardedIteration<LifecycleEventListener>() {
@Override
public void onIterate(LifecycleEventListener listener) {
listener.onHostDestroy();
}
};

private LifecycleState mLifecycleState = LifecycleState.BEFORE_CREATE;

Expand Down Expand Up @@ -179,26 +203,19 @@ public void onHostResume(@Nullable Activity activity) {
mLifecycleState = LifecycleState.RESUMED;
mCurrentActivity = new WeakReference(activity);
ReactMarker.logMarker(ReactMarkerConstants.ON_HOST_RESUME_START);
for (LifecycleEventListener listener : mLifecycleEventListeners) {
try {
listener.onHostResume();
} catch (RuntimeException e) {
handleException(e);
}
}
mLifecycleEventListeners.iterate(mResumeIteration);
ReactMarker.logMarker(ReactMarkerConstants.ON_HOST_RESUME_END);
}

public void onNewIntent(@Nullable Activity activity, Intent intent) {
public void onNewIntent(@Nullable Activity activity, final Intent intent) {
UiThreadUtil.assertOnUiThread();
mCurrentActivity = new WeakReference(activity);
for (ActivityEventListener listener : mActivityEventListeners) {
try {
mActivityEventListeners.iterate(new GuardedIteration<ActivityEventListener>() {
@Override
public void onIterate(ActivityEventListener listener) {
listener.onNewIntent(intent);
} catch (RuntimeException e) {
handleException(e);
}
}
});
}

/**
Expand All @@ -207,13 +224,7 @@ public void onNewIntent(@Nullable Activity activity, Intent intent) {
public void onHostPause() {
mLifecycleState = LifecycleState.BEFORE_RESUME;
ReactMarker.logMarker(ReactMarkerConstants.ON_HOST_PAUSE_START);
for (LifecycleEventListener listener : mLifecycleEventListeners) {
try {
listener.onHostPause();
} catch (RuntimeException e) {
handleException(e);
}
}
mLifecycleEventListeners.iterate(mPauseIteration);
ReactMarker.logMarker(ReactMarkerConstants.ON_HOST_PAUSE_END);
}

Expand All @@ -223,13 +234,7 @@ public void onHostPause() {
public void onHostDestroy() {
UiThreadUtil.assertOnUiThread();
mLifecycleState = LifecycleState.BEFORE_CREATE;
for (LifecycleEventListener listener : mLifecycleEventListeners) {
try {
listener.onHostDestroy();
} catch (RuntimeException e) {
handleException(e);
}
}
mLifecycleEventListeners.iterate(mDestroyIteration);
mCurrentActivity = null;
}

Expand All @@ -247,14 +252,13 @@ public void destroy() {
/**
* Should be called by the hosting Fragment in {@link Fragment#onActivityResult}
*/
public void onActivityResult(Activity activity, int requestCode, int resultCode, Intent data) {
for (ActivityEventListener listener : mActivityEventListeners) {
try {
public void onActivityResult(final Activity activity, final int requestCode, final int resultCode, final Intent data) {
mActivityEventListeners.iterate(new GuardedIteration<ActivityEventListener>() {
@Override
public void onIterate(ActivityEventListener listener) {
listener.onActivityResult(activity, requestCode, resultCode, data);
} catch (RuntimeException e) {
handleException(e);
}
}
});
}

public void assertOnUiQueueThread() {
Expand Down Expand Up @@ -350,4 +354,16 @@ public JavaScriptContextHolder getJavaScriptContextHolder() {
return mCatalystInstance.getJavaScriptContextHolder();
}

private abstract class GuardedIteration<T> implements SynchronizedWeakHashSet.Iteration<T> {
@Override
public void iterate(T listener) {
try {
onIterate(listener);
} catch (RuntimeException e) {
handleException(e);
}
}

public abstract void onIterate(T listener);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// Copyright (c) Facebook, Inc. and its affiliates.

// This source code is licensed under the MIT license found in the
// LICENSE file in the root directory of this source tree.
package com.facebook.react.bridge;

import android.util.Pair;

import java.util.ArrayDeque;
import java.util.Queue;
import java.util.WeakHashMap;

/**
* Thread-safe Set based on the WeakHashMap.
*
* Doesn't implement the `iterator` method because it's tricky to support modifications
* to the collection while somebody is using an `Iterator` to iterate over it.
*
* Instead, it provides an `iterate` method for traversing the collection. Any add/remove operations
* that occur during iteration are postponed until the iteration has completed.
*/
public class SynchronizedWeakHashSet<T> {
private WeakHashMap<T, Void> mMap = new WeakHashMap<>();
private Queue<Pair<T, Command>> mPendingOperations = new ArrayDeque<>();
private boolean mIterating;

public boolean contains(T item) {
synchronized (mMap) {
return mMap.containsKey(item);
}
}

public void add(T item) {
synchronized (mMap) {
if (mIterating) {
mPendingOperations.add(new Pair<>(item, Command.ADD));
} else {
mMap.put(item, null);
}
}
}

public void remove(T item) {
synchronized (mMap) {
if (mIterating) {
mPendingOperations.add(new Pair<>(item, Command.REMOVE));
} else {
mMap.remove(item);
}
}
}

public void iterate(Iteration<T> iterated) {
synchronized (mMap) {
// Protection from modification during iteration on the same thread
mIterating = true;
for (T listener: mMap.keySet()) {
iterated.iterate(listener);
}
mIterating = false;

while (!mPendingOperations.isEmpty()) {
Pair<T, Command> pair = mPendingOperations.poll();
switch (pair.second) {
case ADD:
mMap.put(pair.first, null);
break;
case REMOVE:
mMap.remove(pair.first);
break;
default:
throw new AssertionException("Unsupported command" + pair.second);
}
}
}
}

public interface Iteration<T> {
void iterate(T item);
}

private enum Command {
ADD,
REMOVE
}
}