Change jsinspector back to a shared library in the CMake build (#42144)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42144

D51895785 changed several CMake libraries from shared to static, including `jsinspector`. This happens to be semantically incorrect in the case of `jsinspector`, as the library contains singletons which can be inadvertently duplicated due to static linking. As a result, different parts of the code can end up accessing different instances of a supposed singleton, leading to bugs.

Here we revert the change to `jsinspector` (only) and add an explanatory comment to signpost this for future readers.

## More context & general principle

While nothing is broken today, allowing static libraries to contain global state is brittle and breaks in surprising ways:

* The upcoming diff D52231237 introduces a new dependency on `jsinspector` which builds cleanly, but causes debugging to stop working because of the duplicated singleton.
* The only reason debugging currently works in the CMake build of Bridgeless is by a happy accident: the shared library `hermesinstancejni` depends on `reactnativejni` through a chain of three other libraries unrelated to debugging, and as a result, can access `reactnativejni`'s copy of `jsinspector` (see graph).

 {F1237835169}

It seems that the safest rule of thumb, given the way React Native is currently structured, is that **singletons should live in their own shared libraries** so no call site can cause them to be duplicated through static linking. (It's reasonable to revisit this guidance if we manage to consolidate React Native into one monolithic shared library, eliminating the footgun at the source.)

Changelog:
[Internal] [Changed] - Change jsinspector back to a shared library in the CMake build.

Reviewed By: cortinico, NickGerleman

Differential Revision: D52541488

fbshipit-source-id: 502210add0b734a9bbc470bdf38fb70a41e149a9
This commit is contained in:
Moti Zilberman
2024-01-04 10:01:26 -08:00
committed by Facebook GitHub Bot
parent 5aa425c086
commit 9ba3bc99e4
@@ -11,7 +11,10 @@ add_compile_options(
-std=c++20)
file(GLOB jsinspector_SRC CONFIGURE_DEPENDS *.cpp)
add_library(jsinspector STATIC ${jsinspector_SRC})
# jsinspector contains singletons that hold app-global state (InspectorFlags, InspectorImpl).
# Placing it in a shared library makes the singletons safe to use from arbitrary shared libraries
# (even ones that don't depend on one another).
add_library(jsinspector SHARED ${jsinspector_SRC})
target_include_directories(jsinspector PUBLIC ${REACT_COMMON_DIR})