iOS: create EventTarget when creating EventEmitter and keep it until the emitter is deallocated.

Summary:
@public
There are some race conditions between VM objects getting deallocated and the instanceHandle held by the eventEmitter can point to deallocated memory space, causing undefined behavior like a crash.
For now, keep a strong ref to the eventTarget inside EventEmitter to avoid that scenario. This is a temporary workaround.

Reviewed By: shergin

Differential Revision: D8576785

fbshipit-source-id: 87ef36f716270ceca906b32bb86e0046ceaca19e
This commit is contained in:
Kevin Gozali 2018-06-21 14:14:59 -07:00 committed by Facebook Github Bot
parent 0cca01b16e
commit 20a8673b48
7 changed files with 43 additions and 4 deletions

View File

@ -39,6 +39,9 @@ public:
const folly::dynamic &payload,
const EventPriority &priority
) const = 0;
virtual void releaseEventTarget(const EventTarget &eventTarget) const = 0;
};
} // namespace react

View File

@ -15,9 +15,18 @@ namespace react {
EventEmitter::EventEmitter(const InstanceHandle &instanceHandle, const Tag &tag, const SharedEventDispatcher &eventDispatcher):
instanceHandle_(instanceHandle),
tag_(tag),
eventDispatcher_(eventDispatcher) {}
eventDispatcher_(eventDispatcher) {
if (eventDispatcher) {
eventTarget_ = createEventTarget();
}
}
EventEmitter::~EventEmitter() {}
EventEmitter::~EventEmitter() {
auto &&eventDispatcher = eventDispatcher_.lock();
if (eventDispatcher && eventTarget_) {
eventDispatcher->releaseEventTarget(eventTarget_);
}
}
void EventEmitter::dispatchEvent(
const std::string &type,
@ -29,7 +38,7 @@ void EventEmitter::dispatchEvent(
return;
}
EventTarget eventTarget = createEventTarget();
assert(eventTarget_ && "Attempted to dispatch an event without an eventTarget.");
// Mixing `target` into `payload`.
assert(payload.isObject());
@ -37,7 +46,7 @@ void EventEmitter::dispatchEvent(
extendedPayload.merge_patch(payload);
// TODO(T29610783): Reconsider using dynamic dispatch here.
eventDispatcher->dispatchEvent(eventTarget, type, extendedPayload, priority);
eventDispatcher->dispatchEvent(eventTarget_, type, extendedPayload, priority);
}
EventTarget EventEmitter::createEventTarget() const {

View File

@ -53,6 +53,7 @@ private:
InstanceHandle instanceHandle_;
Tag tag_;
std::weak_ptr<const EventDispatcher> eventDispatcher_;
mutable EventTarget eventTarget_ {nullptr};
};
} // namespace react

View File

@ -102,6 +102,10 @@ void FabricUIManager::setReleaseEventHandlerFunction(std::function<ReleaseEventH
releaseEventHandlerFunction_ = releaseEventHandlerFunction;
}
void FabricUIManager::setReleaseEventTargetFunction(std::function<ReleaseEventTargetFunction> releaseEventTargetFunction) {
releaseEventTargetFunction_ = releaseEventTargetFunction;
}
EventTarget FabricUIManager::createEventTarget(const InstanceHandle &instanceHandle) const {
return createEventTargetFunction_(instanceHandle);
}
@ -115,6 +119,10 @@ void FabricUIManager::dispatchEvent(const EventTarget &eventTarget, const std::s
);
}
void FabricUIManager::releaseEventTarget(const EventTarget &eventTarget) const {
releaseEventTargetFunction_(eventTarget);
}
SharedShadowNode FabricUIManager::createNode(int tag, std::string viewName, int rootTag, folly::dynamic props, InstanceHandle instanceHandle) {
isLoggingEnabled && LOG(INFO) << "FabricUIManager::createNode(tag: " << tag << ", name: " << viewName << ", rootTag: " << rootTag << ", props: " << props << ")";

View File

@ -21,6 +21,7 @@ namespace react {
using CreateEventTargetFunction = EventTarget (InstanceHandle instanceHandle);
using DispatchEventFunction = void (EventHandler eventHandler, EventTarget eventTarget, std::string type, folly::dynamic payload);
using ReleaseEventHandlerFunction = void (EventHandler eventHandler);
using ReleaseEventTargetFunction = void (EventTarget eventTarget);
class FabricUIManager {
public:
@ -46,11 +47,13 @@ public:
void setCreateEventTargetFunction(std::function<CreateEventTargetFunction> createEventTargetFunction);
void setDispatchEventFunction(std::function<DispatchEventFunction> dispatchEventFunction);
void setReleaseEventHandlerFunction(std::function<ReleaseEventHandlerFunction> releaseEventHandlerFunction);
void setReleaseEventTargetFunction(std::function<ReleaseEventTargetFunction> releaseEventTargetFunction);
#pragma mark - Native-facing Interface
EventTarget createEventTarget(const InstanceHandle &instanceHandle) const;
void dispatchEvent(const EventTarget &eventTarget, const std::string &type, const folly::dynamic &payload) const;
void releaseEventTarget(const EventTarget &eventTarget) const;
#pragma mark - JavaScript/React-facing Interface
@ -73,6 +76,7 @@ private:
std::function<CreateEventTargetFunction> createEventTargetFunction_;
std::function<DispatchEventFunction> dispatchEventFunction_;
std::function<ReleaseEventHandlerFunction> releaseEventHandlerFunction_;
std::function<ReleaseEventTargetFunction> releaseEventTargetFunction_;
};
} // namespace react

View File

@ -27,6 +27,7 @@ void SchedulerEventDispatcher::setUIManager(std::shared_ptr<const FabricUIManage
}
EventTarget SchedulerEventDispatcher::createEventTarget(const InstanceHandle &instanceHandle) const {
assert(uiManager_ && "Attempted to create EventTarget after FabricUIManager dies.");
return uiManager_->createEventTarget(instanceHandle);
}
@ -36,9 +37,19 @@ void SchedulerEventDispatcher::dispatchEvent(
const folly::dynamic &payload,
const EventPriority &priority
) const {
if (!uiManager_) {
return;
}
// TODO: Schedule the event based on priority.
uiManager_->dispatchEvent(eventTarget, normalizeEventType(type), payload);
}
void SchedulerEventDispatcher::releaseEventTarget(const EventTarget &eventTarget) const {
if (!uiManager_) {
return;
}
uiManager_->releaseEventTarget(eventTarget);
}
} // namespace react
} // namespace facebook

View File

@ -40,6 +40,9 @@ public:
const EventPriority &priority
) const override;
void releaseEventTarget(const EventTarget &eventTarget) const override;
private:
// TODO: consider using std::weak_ptr<> instead for better memory management.