Skip to content

Commit

Permalink
Fix event handling (#1319)
Browse files Browse the repository at this point in the history
Co-authored-by: karol-bisztyga <[email protected]>
Copied from #1149
## Description

Previously, we were handling dispatched events like so:
```java
@OverRide
 public void onEventDispatch(Event event) {
   if (UiThreadUtil.isOnUiThread()) {
     handleEvent(event);
   } else {
     mEventQueue.offer(event);
     startUpdatingOnAnimationFrame();
    }
  }
```

Event handling in RN works by utilizing `EventDispatcher.dispatchEvent(event)` which
1. runs `onEventDispatch` callback on any registered listener
2. adds `event` to internal event queue
3. adds frame callback which dispatches and disposes events on JS thread

This approach introduced timing issues - RN's `EventDispatcher` dispatches events on JS thread and Reanimated handles events on UI thread. There's a possibility that `EventDispatcher` will dispose event (possibly destroying it's state in `onDispose()`) before Reanimated would have chance to handle it.
This was found after investigating [pretty popular crash in react-native-gesture-handler](software-mansion/react-native-gesture-handler#1171).

# HOW
The pull-request adds another method `isAnyHandlerWaitingForEvent` to NativeProxy API which lets us check if an event is important (there is workletHandler listening for the event) or not. The rest part of the pr is very similar to Jakub's pr. However, there are some differences. Instead of saving copied event Object, we save: tag, eventName, and payload in the new class `CopiedEvent`.
  • Loading branch information
Szymon20000 authored Oct 27, 2020
1 parent 7685287 commit c79d27c
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 4 deletions.
5 changes: 5 additions & 0 deletions Common/cpp/NativeModules/NativeReanimatedModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,11 @@ void NativeReanimatedModule::onEvent(std::string eventName, std::string eventAsS
}
}

bool NativeReanimatedModule::isAnyHandlerWaitingForEvent(std::string eventName) {
return eventHandlerRegistry->isAnyHandlerWaitingForEvent(eventName);
}


void NativeReanimatedModule::maybeRequestRender()
{
if (!renderRequested)
Expand Down
8 changes: 8 additions & 0 deletions Common/cpp/Registries/EventHandlerRegistry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,21 @@
namespace reanimated {

void EventHandlerRegistry::registerEventHandler(std::shared_ptr<EventHandler> eventHandler) {
const std::lock_guard<std::mutex> lock(instanceMutex);
eventMappings[eventHandler->eventName][eventHandler->id] = eventHandler;
eventHandlers[eventHandler->id] = eventHandler;
}

void EventHandlerRegistry::unregisterEventHandler(unsigned long id) {
const std::lock_guard<std::mutex> lock(instanceMutex);
auto handlerIt = eventHandlers.find(id);
if (handlerIt != eventHandlers.end()) {
eventMappings[handlerIt->second->eventName].erase(id);
}
}

void EventHandlerRegistry::processEvent(jsi::Runtime &rt, std::string eventName, std::string eventPayload) {
const std::lock_guard<std::mutex> lock(instanceMutex);
auto handlersIt = eventMappings.find(eventName);
if (handlersIt != eventMappings.end()) {
// We receive here a JS Map with JSON as a value of NativeMap key
Expand All @@ -35,4 +38,9 @@ void EventHandlerRegistry::processEvent(jsi::Runtime &rt, std::string eventName,
}
}

bool EventHandlerRegistry::isAnyHandlerWaitingForEvent(std::string eventName) {
auto it = eventMappings.find(eventName);
return (it != eventMappings.end()) and (!(it->second).empty());
}

}
1 change: 1 addition & 0 deletions Common/cpp/headers/NativeModules/NativeReanimatedModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class NativeReanimatedModule : public NativeReanimatedModuleSpec

void onRender(double timestampMs);
void onEvent(std::string eventName, std::string eventAsString);
bool isAnyHandlerWaitingForEvent(std::string eventName);

void maybeRequestRender();

Expand Down
3 changes: 3 additions & 0 deletions Common/cpp/headers/Registries/EventHandlerRegistry.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <unordered_map>
#include <string>
#include <jsi/jsi.h>
#include <mutex>

using namespace facebook;

Expand All @@ -16,12 +17,14 @@ class EventHandler;
class EventHandlerRegistry {
std::map<std::string, std::unordered_map<unsigned long, std::shared_ptr<EventHandler>>> eventMappings;
std::map<unsigned long, std::shared_ptr<EventHandler>> eventHandlers;
std::mutex instanceMutex;

public:
void registerEventHandler(std::shared_ptr<EventHandler> eventHandler);
void unregisterEventHandler(unsigned long id);

void processEvent(jsi::Runtime &rt, std::string eventName, std::string eventPayload);
bool isAnyHandlerWaitingForEvent(std::string eventName);
};

}
8 changes: 7 additions & 1 deletion android/src/main/cpp/NativeProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#include "NativeProxy.h"
#include "AndroidErrorHandler.h"
#include "NativeReanimatedModule.h"
#include "AndroidScheduler.h"
#include <android/log.h>
#include "PlatformDepMethodsHolder.h"
Expand Down Expand Up @@ -91,6 +90,8 @@ void NativeProxy::installJSIBindings()
propObtainer,
platformDepMethodsHolder);

_nativeReanimatedModule = module;

this->registerEventHandler([module](std::string eventName, std::string eventAsString) {
module->onEvent(eventName, eventAsString);
});
Expand All @@ -101,11 +102,16 @@ void NativeProxy::installJSIBindings()
jsi::Object::createFromHostObject(*runtime_, module));
}

bool NativeProxy::isAnyHandlerWaitingForEvent(std::string s) {
return _nativeReanimatedModule->isAnyHandlerWaitingForEvent(s);
}

void NativeProxy::registerNatives()
{
registerHybrid({
makeNativeMethod("initHybrid", NativeProxy::initHybrid),
makeNativeMethod("installJSIBindings", NativeProxy::installJSIBindings),
makeNativeMethod("isAnyHandlerWaitingForEvent", NativeProxy::isAnyHandlerWaitingForEvent)
});
}

Expand Down
4 changes: 4 additions & 0 deletions android/src/main/cpp/headers/NativeProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
#include <react/jni/CxxModuleWrapper.h>
#include <react/jni/JMessageQueueThread.h>
#include <react/jni/WritableNativeMap.h>
#include "NativeReanimatedModule.h"
#include <ReactCommon/CallInvokerHolder.h>
#include <memory>
#include <unordered_map>


#include "Scheduler.h"
#include "AndroidScheduler.h"

Expand Down Expand Up @@ -89,9 +91,11 @@ class NativeProxy : public jni::HybridClass<NativeProxy> {
jni::global_ref<NativeProxy::javaobject> javaPart_;
jsi::Runtime *runtime_;
std::shared_ptr<facebook::react::CallInvoker> jsCallInvoker_;
std::shared_ptr<NativeReanimatedModule> _nativeReanimatedModule;
std::shared_ptr<Scheduler> scheduler_;

void installJSIBindings();
bool isAnyHandlerWaitingForEvent(std::string);
void requestRender(std::function<void(double)> onRender);
void registerEventHandler(std::function<void(std::string,std::string)> handler);
void updateProps(jsi::Runtime &rt, int viewTag, const jsi::Object &props);
Expand Down
42 changes: 42 additions & 0 deletions android/src/main/java/com/swmansion/reanimated/CopiedEvent.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.swmansion.reanimated;

import com.facebook.react.bridge.WritableArray;
import com.facebook.react.bridge.WritableMap;
import com.facebook.react.uimanager.events.Event;
import com.facebook.react.uimanager.events.RCTEventEmitter;

import androidx.annotation.Nullable;

public class CopiedEvent {
private int targetTag;
private String eventName;
private WritableMap payload;

CopiedEvent(Event event) {
event.dispatch(new RCTEventEmitter() {
@Override
public void receiveEvent(int targetTag, String eventName, @Nullable WritableMap event) {
CopiedEvent.this.targetTag = targetTag;
CopiedEvent.this.eventName = eventName;
CopiedEvent.this.payload = event.copy();
}

@Override
public void receiveTouches(String eventName, WritableArray touches, WritableArray changedIndices) {
//noop
}
});
}

public int getTargetTag() {
return targetTag;
}

public String getEventName() {
return eventName;
}

public WritableMap getPayload() {
return payload;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ public NativeProxy(ReactApplicationContext context) {
private native HybridData initHybrid(long jsContext, CallInvokerHolderImpl jsCallInvokerHolder, Scheduler scheduler);
private native void installJSIBindings();

public native boolean isAnyHandlerWaitingForEvent(String eventName);

@DoNotStrip
private void requestRender(AnimationFrameCallback callback) {
mNodesManager.postOnAnimation(callback);
Expand Down
32 changes: 29 additions & 3 deletions android/src/main/java/com/swmansion/reanimated/NodesManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@
import java.util.concurrent.Semaphore;
import java.util.concurrent.atomic.AtomicBoolean;

import javax.annotation.Nullable;

public class NodesManager implements EventDispatcherListener {

private static final Double ZERO = Double.valueOf(0);
Expand Down Expand Up @@ -104,7 +106,7 @@ public interface OnAnimationFrame {

private RCTEventEmitter mCustomEventHandler;
private List<OnAnimationFrame> mFrameCallbacks = new ArrayList<>();
private ConcurrentLinkedQueue<Event> mEventQueue = new ConcurrentLinkedQueue<>();
private ConcurrentLinkedQueue<CopiedEvent> mEventQueue = new ConcurrentLinkedQueue<>();
private boolean mWantRunUpdates;

public double currentFrameTimeMs;
Expand Down Expand Up @@ -228,7 +230,8 @@ private void onAnimationFrame(long frameTimeNanos) {
currentFrameTimeMs = frameTimeNanos / 1000000.;

while (!mEventQueue.isEmpty()) {
handleEvent(mEventQueue.poll());
CopiedEvent copiedEvent = mEventQueue.poll();
handleEvent(copiedEvent.getTargetTag(), copiedEvent.getEventName(), copiedEvent.getPayload());
}

if (!mFrameCallbacks.isEmpty()) {
Expand Down Expand Up @@ -445,7 +448,15 @@ public void onEventDispatch(Event event) {
handleEvent(event);
performOperations();
} else {
mEventQueue.offer(event);
boolean shouldSaveEvent = false;
String eventName = mCustomEventNamesResolver.resolveCustomEventName(event.getEventName());
int viewTag = event.getViewTag();
String key = viewTag + eventName;

shouldSaveEvent |= (mCustomEventHandler != null && mNativeProxy.isAnyHandlerWaitingForEvent(key));
if (shouldSaveEvent) {
mEventQueue.offer(new CopiedEvent(event));
}
startUpdatingOnAnimationFrame();
}
}
Expand All @@ -468,6 +479,21 @@ private void handleEvent(Event event) {
}
}

private void handleEvent(int targetTag, String eventName, @Nullable WritableMap event) {
if (mCustomEventHandler != null) {
mCustomEventHandler.receiveEvent(targetTag, eventName, event);
}

String key = targetTag + eventName;

if (!mEventMapping.isEmpty()) {
EventNode node = mEventMapping.get(key);
if (node != null) {
node.receiveEvent(targetTag, eventName, event);
}
}
}

public UIManagerModule.CustomEventNamesResolver getEventNameResolver() {
return mCustomEventNamesResolver;
}
Expand Down

0 comments on commit c79d27c

Please sign in to comment.