From cdb983d3392d4339551b18a572aea52b72df0d9d Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Wed, 16 Jan 2019 20:17:01 -0800 Subject: [PATCH] Fabric: Lock-free events 4/n: Added an assert in EventTarget Summary: See the diff for more details. Reviewed By: sahrens Differential Revision: D13642593 fbshipit-source-id: 8bdcc91bcc2ea1e4093bcac03d87167b8901cbb4 --- ReactCommon/fabric/events/EventEmitter.h | 2 +- ReactCommon/fabric/events/EventTarget.cpp | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/ReactCommon/fabric/events/EventEmitter.h b/ReactCommon/fabric/events/EventEmitter.h index 8e3045170..3e06c81e4 100644 --- a/ReactCommon/fabric/events/EventEmitter.h +++ b/ReactCommon/fabric/events/EventEmitter.h @@ -46,7 +46,6 @@ class EventEmitter { virtual ~EventEmitter() = default; /* - * `DispatchMutex` must be acquired before calling. * Enables/disables event emitter. * Enabled event emitter retains a pointer to `eventTarget` strongly (as * `std::shared_ptr`) whereas disabled one don't. @@ -54,6 +53,7 @@ class EventEmitter { * a possibility to extract JSI value from it. * The enable state is additive; a number of `enable` calls should be equal to * a number of `disable` calls to release the event target. + * `DispatchMutex` must be acquired before calling. */ void setEnabled(bool enabled) const; diff --git a/ReactCommon/fabric/events/EventTarget.cpp b/ReactCommon/fabric/events/EventTarget.cpp index ee893d631..64dc25490 100644 --- a/ReactCommon/fabric/events/EventTarget.cpp +++ b/ReactCommon/fabric/events/EventTarget.cpp @@ -31,6 +31,15 @@ void EventTarget::retain(jsi::Runtime &runtime) const { } strongInstanceHandle_ = weakInstanceHandle_.lock(runtime); + + // Having a `null` or `undefined` object here indicates that + // `weakInstanceHandle_` was already deallocated. This should *not* happen by + // design, and if it happens it's a severe problem. This basically means that + // particular implementation of JSI was able to detect this inconsistency and + // dealt with it, but some JSI implementation may not support this feature and + // that case will lead to a crash in those environments. + assert(!strongInstanceHandle_.isNull()); + assert(!strongInstanceHandle_.isUndefined()); } jsi::Value EventTarget::release(jsi::Runtime &runtime) const {