-
Notifications
You must be signed in to change notification settings - Fork 24.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This change fixes currently broken ReactContext listeners mechanism. (#…
…22318) Summary: This mechanism is heavily abused inside of the react-native and inside of the various native modules. The main problem is that people don't remove their listeners and as result, we have memory leaks. Some modules like UIManager, NativeAnimatedModule have resources holding Activity context. Those modules are held through a pretty long chain of dependencies. In order to allow GC to collect those listeners, I replaced the CopyOnWriteSet by WeakHashMap and synchronized access. It is not such a big deal in terms of performance as those listeners are not called/modified too frequently but this prevents hard to debug memory leaks. Changelog: ---------- Help reviewers and the release process by writing your own changelog entry. When the change doesn't impact React Native developers, it may be ommitted from the changelog for brevity. See below for an example. [Android] [Fixed] - ReactContext - lifecycle listeners don't cause the leaks even if not removed. Pull Request resolved: #22318 Reviewed By: mdvacca Differential Revision: D13106915 Pulled By: hramos fbshipit-source-id: d506e5035a7f7bea1b57a6308fb5d9b5fcb277a7
- Loading branch information
1 parent
b2d3052
commit 972f399
Showing
2 changed files
with
140 additions
and
38 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
86 changes: 86 additions & 0 deletions
86
ReactAndroid/src/main/java/com/facebook/react/bridge/SynchronizedWeakHashSet.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 | ||
| } | ||
| } |