Skip to content

Commit

Permalink
Fix initialisation of NodesManager (#1030)
Browse files Browse the repository at this point in the history
## Description

Previously, `NodesManager` registered (passing `this`) as an event listener before ending initialization in c-tor.


`NodesManager` is created by `ReanimatedModule` (when calling `getNodesManager()` for the first time) on [UI thread](https://github.com/software-mansion/react-native-reanimated/blob/9af276693d136b712259fe9dd0126aee6c78bd4b/android/src/main/java/com/swmansion/reanimated/ReanimatedModule.java#L77-L80).

When `onEventDispatch()` is called on non-UI thread it calls `startUpdatingOnAnimationFrame()`, which in turn uses `mChoreographerCallback`.
`mChoreographerCallback` is initialised **after** listener is registered which causes NPE in React Choreographer's `postFrameCallback()` method after trying adding callback to the queue.

Fixes #604.

References:
https://www.ibm.com/developerworks/java/library/j-jtp0618/index.html#2
https://stackoverflow.com/questions/25281301/what-exactly-are-the-dangers-of-passing-this-from-a-java-constructor
  • Loading branch information
jakub-gonet authored Aug 11, 2020
1 parent 9af2766 commit d3b6039
Showing 1 changed file with 6 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ public NodesManager(ReactContext context) {
updateContext = new UpdateContext();
mUIImplementation = mUIManager.getUIImplementation();
mCustomEventNamesResolver = mUIManager.getDirectEventNamesResolver();
mUIManager.getEventDispatcher().addListener(this);

mEventEmitter = context.getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class);

mReactChoreographer = ReactChoreographer.getInstance();
Expand All @@ -110,6 +108,12 @@ protected void doFrameGuarded(long frameTimeNanos) {
};

mNoopNode = new NoopNode(this);

// We register as event listener at the end, because we pass `this` and we haven't finished contructing an object yet.
// This lead to a crash described in https://github.com/software-mansion/react-native-reanimated/issues/604 which was caused by Nodes Manager being constructed on UI thread and registering for events.
// Events are handled in the native modules thread in the `onEventDispatch()` method.
// This method indirectly uses `mChoreographerCallback` which was created after event registration, creating race condition
mUIManager.getEventDispatcher().addListener(this);
}

public void onHostPause() {
Expand Down

0 comments on commit d3b6039

Please sign in to comment.