Fix TSAN data races in InspectorFlags and InspectorPackagerConnection (#56534)

Summary:
Pull Request resolved: #56534

Fix two ThreadSanitizer data races caught by the messenger_demo integration test under dev-tsan mode.

**InspectorFlags**: `loadFlagsAndAssertUnchanged()` reads and writes `cachedValues_` without synchronization. The main thread calls it via `getIsProfilingBuild()` while the JS thread calls it via `getNetworkInspectionEnabled()`, racing on the shared `optional<Values>`. Add a `std::mutex` to protect `cachedValues_` access. The lock is taken after computing `newValues` from feature flags, so flag reads stay outside the critical section. `dangerouslyResetFlags()` also acquires the lock to avoid racing with concurrent flag reads.

**Inspector**: `Inspector::connectDebugger()` is called from the main thread but creates and uses `packagerConnection_` which is also accessed on the inspector thread. Dispatch the body of `connectDebugger()` to the task dispatch thread via `invokeElsePost()`, matching the pattern other Inspector methods already use. This ensures `InspectorPackagerConnection::connect()` is called on the correct thread as its API requires.

Changelog: [Internal]

Reviewed By: hoxyq

Differential Revision: D101809616

fbshipit-source-id: 92eac22ca9ab364daafc403a0fdbb39394bd0f63

Author: javache

packages/react-native/ReactCommon/jsinspector-modern/InspectorFlags.cpp

@@ -50,7 +50,10 @@ bool InspectorFlags::getPerfIssuesEnabled() const {
 }
 
 void InspectorFlags::dangerouslyResetFlags() {
-  *this = InspectorFlags{};
+  std::lock_guard<std::mutex> lock(mutex_);
+  cachedValues_.reset();
+  inconsistentFlagsStateLogged_ = false;
+  fuseboxDisabledForTest_ = false;
 }
 
 void InspectorFlags::dangerouslyDisableFuseboxForTest() {
@@ -84,6 +87,8 @@ const InspectorFlags::Values& InspectorFlags::loadFlagsAndAssertUnchanged()
       .perfIssuesEnabled = ReactNativeFeatureFlags::perfIssuesEnabled(),
   };
 
+  // Protect against concurrent calls
+  std::lock_guard<std::mutex> lock(mutex_);
   if (cachedValues_.has_value() && !inconsistentFlagsStateLogged_) {
     if (cachedValues_ != newValues) {
       LOG(ERROR)
@@ -93,7 +98,6 @@ const InspectorFlags::Values& InspectorFlags::loadFlagsAndAssertUnchanged()
       inconsistentFlagsStateLogged_ = true;
     }
   }
-
   cachedValues_ = newValues;
 
   return cachedValues_.value();

packages/react-native/ReactCommon/jsinspector-modern/InspectorFlags.h

@@ -7,6 +7,7 @@
 
 #pragma once
 
+#include <mutex>
 #include <optional>
 
 namespace facebook::react::jsinspector_modern {
@@ -19,6 +20,9 @@ class InspectorFlags {
  public:
   static InspectorFlags &getInstance();
 
+  InspectorFlags(const InspectorFlags &) = delete;
+  InspectorFlags &operator=(const InspectorFlags &) = delete;
+
   /**
    * Flag determining if the inspector backend should strictly assert that only
    * a single host is registered.
@@ -81,10 +85,9 @@ class InspectorFlags {
   };
 
   InspectorFlags() = default;
-  InspectorFlags(const InspectorFlags &) = delete;
-  InspectorFlags &operator=(const InspectorFlags &) = default;
   ~InspectorFlags() = default;
 
+  mutable std::mutex mutex_;
   mutable std::optional<Values> cachedValues_;
   mutable bool inconsistentFlagsStateLogged_{false};
   bool fuseboxDisabledForTest_{false};

packages/react-native/ReactCxxPlatform/react/devsupport/inspector/Inspector.cpp

@@ -148,18 +148,24 @@ Inspector::~Inspector() noexcept {
 }
 
 void Inspector::connectDebugger(const std::string& inspectorUrl) noexcept {
-  if (!packagerConnection_) {
-    packagerConnection_ =
-        std::make_unique<jsinspector_modern::InspectorPackagerConnection>(
-            inspectorUrl,
-            deviceName_,
-            appName_,
-            std::make_unique<InspectorPackagerConnectionDelegate>(
-                weak_from_this(), webSocketClientFactory_));
-  }
-  if (!packagerConnection_->isConnected()) {
-    packagerConnection_->connect();
-  }
+  invokeElsePost([weakThis = weak_from_this(), inspectorUrl]() {
+    auto self = weakThis.lock();
+    if (!self) {
+      return;
+    }
+    if (!self->packagerConnection_) {
+      self->packagerConnection_ =
+          std::make_unique<jsinspector_modern::InspectorPackagerConnection>(
+              inspectorUrl,
+              self->deviceName_,
+              self->appName_,
+              std::make_unique<InspectorPackagerConnectionDelegate>(
+                  weakThis, self->webSocketClientFactory_));
+    }
+    if (!self->packagerConnection_->isConnected()) {
+      self->packagerConnection_->connect();
+    }
+  });
 }
 
 void Inspector::ensureHostTarget(

scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api

@@ -10528,13 +10528,15 @@ class facebook::react::jsinspector_modern::IWebSocketDelegate {
 }
 
 class facebook::react::jsinspector_modern::InspectorFlags {
+  public InspectorFlags(const facebook::react::jsinspector_modern::InspectorFlags&) = delete;
   public bool getAssertSingleHostState() const;
   public bool getFrameRecordingEnabled() const;
   public bool getFuseboxEnabled() const;
   public bool getIsProfilingBuild() const;
   public bool getNetworkInspectionEnabled() const;
   public bool getPerfIssuesEnabled() const;
   public bool getScreenshotCaptureEnabled() const;
+  public facebook::react::jsinspector_modern::InspectorFlags& operator=(const facebook::react::jsinspector_modern::InspectorFlags&) = delete;
   public static facebook::react::jsinspector_modern::InspectorFlags& getInstance();
   public void dangerouslyDisableFuseboxForTest();
   public void dangerouslyResetFlags();

scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api

@@ -10384,13 +10384,15 @@ class facebook::react::jsinspector_modern::IWebSocketDelegate {
 }
 
 class facebook::react::jsinspector_modern::InspectorFlags {
+  public InspectorFlags(const facebook::react::jsinspector_modern::InspectorFlags&) = delete;
   public bool getAssertSingleHostState() const;
   public bool getFrameRecordingEnabled() const;
   public bool getFuseboxEnabled() const;
   public bool getIsProfilingBuild() const;
   public bool getNetworkInspectionEnabled() const;
   public bool getPerfIssuesEnabled() const;
   public bool getScreenshotCaptureEnabled() const;
+  public facebook::react::jsinspector_modern::InspectorFlags& operator=(const facebook::react::jsinspector_modern::InspectorFlags&) = delete;
   public static facebook::react::jsinspector_modern::InspectorFlags& getInstance();
   public void dangerouslyDisableFuseboxForTest();
   public void dangerouslyResetFlags();

scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api

@@ -12809,13 +12809,15 @@ class facebook::react::jsinspector_modern::IWebSocketDelegate {
 }
 
 class facebook::react::jsinspector_modern::InspectorFlags {
+  public InspectorFlags(const facebook::react::jsinspector_modern::InspectorFlags&) = delete;
   public bool getAssertSingleHostState() const;
   public bool getFrameRecordingEnabled() const;
   public bool getFuseboxEnabled() const;
   public bool getIsProfilingBuild() const;
   public bool getNetworkInspectionEnabled() const;
   public bool getPerfIssuesEnabled() const;
   public bool getScreenshotCaptureEnabled() const;
+  public facebook::react::jsinspector_modern::InspectorFlags& operator=(const facebook::react::jsinspector_modern::InspectorFlags&) = delete;
   public static facebook::react::jsinspector_modern::InspectorFlags& getInstance();
   public void dangerouslyDisableFuseboxForTest();
   public void dangerouslyResetFlags();

scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api

@@ -12675,13 +12675,15 @@ class facebook::react::jsinspector_modern::IWebSocketDelegate {
 }
 
 class facebook::react::jsinspector_modern::InspectorFlags {
+  public InspectorFlags(const facebook::react::jsinspector_modern::InspectorFlags&) = delete;
   public bool getAssertSingleHostState() const;
   public bool getFrameRecordingEnabled() const;
   public bool getFuseboxEnabled() const;
   public bool getIsProfilingBuild() const;
   public bool getNetworkInspectionEnabled() const;
   public bool getPerfIssuesEnabled() const;
   public bool getScreenshotCaptureEnabled() const;
+  public facebook::react::jsinspector_modern::InspectorFlags& operator=(const facebook::react::jsinspector_modern::InspectorFlags&) = delete;
   public static facebook::react::jsinspector_modern::InspectorFlags& getInstance();
   public void dangerouslyDisableFuseboxForTest();
   public void dangerouslyResetFlags();

scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api

@@ -7636,13 +7636,15 @@ class facebook::react::jsinspector_modern::IWebSocketDelegate {
 }
 
 class facebook::react::jsinspector_modern::InspectorFlags {
+  public InspectorFlags(const facebook::react::jsinspector_modern::InspectorFlags&) = delete;
   public bool getAssertSingleHostState() const;
   public bool getFrameRecordingEnabled() const;
   public bool getFuseboxEnabled() const;
   public bool getIsProfilingBuild() const;
   public bool getNetworkInspectionEnabled() const;
   public bool getPerfIssuesEnabled() const;
   public bool getScreenshotCaptureEnabled() const;
+  public facebook::react::jsinspector_modern::InspectorFlags& operator=(const facebook::react::jsinspector_modern::InspectorFlags&) = delete;
   public static facebook::react::jsinspector_modern::InspectorFlags& getInstance();
   public void dangerouslyDisableFuseboxForTest();
   public void dangerouslyResetFlags();

scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api

@@ -7627,13 +7627,15 @@ class facebook::react::jsinspector_modern::IWebSocketDelegate {
 }
 
 class facebook::react::jsinspector_modern::InspectorFlags {
+  public InspectorFlags(const facebook::react::jsinspector_modern::InspectorFlags&) = delete;
   public bool getAssertSingleHostState() const;
   public bool getFrameRecordingEnabled() const;
   public bool getFuseboxEnabled() const;
   public bool getIsProfilingBuild() const;
   public bool getNetworkInspectionEnabled() const;
   public bool getPerfIssuesEnabled() const;
   public bool getScreenshotCaptureEnabled() const;
+  public facebook::react::jsinspector_modern::InspectorFlags& operator=(const facebook::react::jsinspector_modern::InspectorFlags&) = delete;
   public static facebook::react::jsinspector_modern::InspectorFlags& getInstance();
   public void dangerouslyDisableFuseboxForTest();
   public void dangerouslyResetFlags();