PR comments

facebook · Jul 28, 2024 · a67b61b · a67b61b
1 parent 2b881b9
commit a67b61b
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 26 deletions.
diff --git a/packages/react-native/React/CxxBridge/RCTCxxBridge.mm b/packages/react-native/React/CxxBridge/RCTCxxBridge.mm
@@ -162,12 +162,13 @@ static void mapReactMarkerToPerformanceLogger(
 
 static void registerPerformanceLoggerHooks(RCTPerformanceLogger *performanceLogger)
 {
+  std::unique_lock lock(ReactMarker::logTaggedMarkerImplMutex);
   __weak RCTPerformanceLogger *weakPerformanceLogger = performanceLogger;
   ReactMarker::LogTaggedMarker newMarker = [weakPerformanceLogger](
                                          const ReactMarker::ReactMarkerId markerId, const char *tag) {
     mapReactMarkerToPerformanceLogger(markerId, weakPerformanceLogger, tag);
   };
-  ReactMarker::LogTaggedMarkerWrapper::setLogTaggedMarkerImpl(newMarker);
+  ReactMarker::logTaggedMarkerImpl = newMarker;
 }
 
 @interface RCTCxxBridge () <RCTModuleDataCallInvokerProvider>

diff --git a/packages/react-native/ReactCommon/cxxreact/ReactMarker.cpp b/packages/react-native/ReactCommon/cxxreact/ReactMarker.cpp
@@ -17,8 +17,8 @@ namespace ReactMarker {
 #endif
 
 LogTaggedMarker logTaggedMarkerBridgelessImpl = nullptr;
-LogTaggedMarker LogTaggedMarkerWrapper::logTaggedMarkerImpl = nullptr;
-std::shared_mutex LogTaggedMarkerWrapper::logTaggedMarkerImplMutex;
+LogTaggedMarker logTaggedMarkerImpl = nullptr;
+std::shared_mutex logTaggedMarkerImplMutex;
 
 #if __clang__
 #pragma clang diagnostic pop
@@ -29,7 +29,14 @@ void logMarker(const ReactMarkerId markerId) {
 }
 
 void logTaggedMarker(const ReactMarkerId markerId, const char* tag) {
-  LogTaggedMarkerWrapper::getLogTaggedMarkerImpl()(markerId, tag);
+  LogTaggedMarker marker = nullptr;
+  {
+    std::shared_lock lock(logTaggedMarkerImplMutex);
+    marker = logTaggedMarkerImpl;
+  }
+  if (marker) {
+    marker(markerId, tag);
+  }
 }
 
 void logMarkerBridgeless(const ReactMarkerId markerId) {

diff --git a/packages/react-native/ReactCommon/cxxreact/ReactMarker.h b/packages/react-native/ReactCommon/cxxreact/ReactMarker.h
@@ -52,24 +52,9 @@ typedef void (*LogTaggedMarkerBridgeless)(const ReactMarkerId, const char* tag);
 #define RN_EXPORT __attribute__((visibility("default")))
 #endif
 
-class RN_EXPORT LogTaggedMarkerWrapper {
-public:
-  static inline LogTaggedMarker getLogTaggedMarkerImpl(void) {
-    std::shared_lock lock(logTaggedMarkerImplMutex);
-    return logTaggedMarkerImpl;
-  }
-
-  static inline void setLogTaggedMarkerImpl(LogTaggedMarker marker) {
-    std::unique_lock lock(logTaggedMarkerImplMutex);
-    logTaggedMarkerImpl = marker;
-  }
-
-private:
-  LogTaggedMarkerWrapper() = delete;
-  static std::shared_mutex logTaggedMarkerImplMutex;
-  static LogTaggedMarker logTaggedMarkerImpl;
-};
-
+extern RN_EXPORT std::shared_mutex logTaggedMarkerImplMutex;
+/// - important: To ensure this gets read and written to in a thread safe manner, make use of `logTaggedMarkerImplMutex`.
+extern RN_EXPORT LogTaggedMarker logTaggedMarkerImpl;
 extern RN_EXPORT LogTaggedMarker logTaggedMarkerBridgelessImpl;
 
 extern RN_EXPORT void logMarker(const ReactMarkerId markerId); // Bridge only

diff --git a/packages/react-native/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp b/packages/react-native/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp
@@ -139,7 +139,11 @@ void JSIExecutor::initializeRuntime() {
   if (runtimeInstaller_) {
     runtimeInstaller_(*runtime_);
   }
-  bool hasLogger(ReactMarker::LogTaggedMarkerWrapper::getLogTaggedMarkerImpl());
+  bool hasLogger = false;
+  {
+    std::shared_lock lock(ReactMarker::logTaggedMarkerImplMutex);
+    hasLogger = ReactMarker::logTaggedMarkerImpl != nullptr;
+  }
   if (hasLogger) {
     ReactMarker::logMarker(ReactMarker::CREATE_REACT_CONTEXT_STOP);
   }
@@ -149,8 +153,11 @@ void JSIExecutor::loadBundle(
     std::unique_ptr<const JSBigString> script,
     std::string sourceURL) {
   SystraceSection s("JSIExecutor::loadBundle");
-
-  bool hasLogger(ReactMarker::LogTaggedMarkerWrapper::getLogTaggedMarkerImpl());
+  bool hasLogger = false;
+  {
+    std::shared_lock lock(ReactMarker::logTaggedMarkerImplMutex);
+    hasLogger = ReactMarker::logTaggedMarkerImpl != nullptr;
+  }
   std::string scriptName = simpleBasename(sourceURL);
   if (hasLogger) {
     ReactMarker::logTaggedMarker(

diff --git a/packages/react-native/ReactCommon/jsiexecutor/jsireact/JSINativeModules.cpp b/packages/react-native/ReactCommon/jsiexecutor/jsireact/JSINativeModules.cpp
@@ -67,7 +67,11 @@ void JSINativeModules::reset() {
 std::optional<Object> JSINativeModules::createModule(
     Runtime& rt,
     const std::string& name) {
-  bool hasLogger(ReactMarker::LogTaggedMarkerWrapper::getLogTaggedMarkerImpl());
+  bool hasLogger = false;
+  {
+    std::shared_lock lock(ReactMarker::logTaggedMarkerImplMutex);
+    hasLogger = ReactMarker::logTaggedMarkerImpl != nullptr;
+  }
   if (hasLogger) {
     ReactMarker::logTaggedMarker(
         ReactMarker::NATIVE_MODULE_SETUP_START, name.c_str());