diff --git a/.claude/skills/build-compilation-dependencies/SKILL.md b/.claude/skills/build-compilation-dependencies/SKILL.md index 405fecf3e..1ed21e913 100644 --- a/.claude/skills/build-compilation-dependencies/SKILL.md +++ b/.claude/skills/build-compilation-dependencies/SKILL.md @@ -340,6 +340,8 @@ yarn workspace react-native-audio-api test:cpp:extended -- graph `run-tests.sh [smoke|extended|full] [category…] [--ubasan|--tsan|--no-ubasan]` uses filters from `filters.sh`. Docs: `common/cpp/test/TESTING.md`. Shell scripts in this repo use kebab-case plus `.sh` (`run-tests.sh`, not `RunTests.sh`). +> **Sanitizer binaries compile every engine translation unit with that sanitizer.** `add_sanitized_tests( )` builds `tests_` plus `rnaudioapi_` and `rnaudioapi_libs_`. `tsan` uses `-fsanitize=thread`; `asan` uses `-fsanitize=address,undefined`. The plain `rnaudioapi` pair is only for `tests` and the coverage build. TSan misses release/acquire pairs in uninstrumented code, so an uninstrumented engine turns every SPSC hand-off into a false race (`` frames, "As if synchronized via sleep"). ASan and UBSan simply never check a translation unit they did not compile. `AudioThreadGuard.cpp` is compiled into the executable, not the static lib, so its `__SANITIZE_*` macros match that binary. GoogleTest stays the shared uninstrumented target: one gtest cannot be compiled with both sanitizers and with neither. + ### Coverage (Clang / llvm-cov) ```bash @@ -356,9 +358,9 @@ CI runs a parallel `cpp-coverage` job via `.github/workflows/cpp-coverage-job.ym ### Key design decisions - Completely standalone — no Gradle, no Xcode, no prebuilt Android libraries needed - Sources resolved from `node_modules` (symlinked to `packages/` in yarn workspaces) -- HostObjects, worklets nodes, AudioContext, and FfmpegDecoder are excluded from the test build +- HostObjects (except `JsEnumParser.cpp`), AudioContext, and FfmpegDecoder are excluded from the test build. There is no worklets-node filter and no `RN_AUDIO_API_ENABLE_WORKLETS` define; engine sources no longer read that flag - HostObject **headers** still compile under `RN_AUDIO_API_TEST`; only their `.cpp` bodies are missing. When a test first links core code that constructs one (e.g. `AudioRecorderCallback` → `AudioBufferHostObject`), the linker fails on the constructor. Fix it with a stub translation unit under `test/src/` that defines just the constructors (see `test/src/AudioBufferHostObjectStub.cpp`), never with an `#if RN_AUDIO_API_TEST` test double inside the production header. The static lib also compiles `jsi/jsi.cpp` so `jsi::HostObject`'s virtuals resolve -- Compile definitions: `RN_AUDIO_API_ENABLE_WORKLETS=0`, `RN_AUDIO_API_TEST=1` (FFmpeg is off because the define is simply absent) +- Compile definitions: `RN_AUDIO_API_TEST=1`, `RN_AUDIO_API_STATIC_EXTERNAL_LIBS_DISABLED=1`, `MA_NO_LIBOPUS=1`, `MA_NO_LIBVORBIS=1` (FFmpeg is off because its define is absent). `add_compile_definitions` is directory-scoped, so every target in this file receives them - Google Test auto-fetched via `FetchContent` if not installed locally - New test files in `test/src/**/*.cpp` are picked up automatically by glob — no CMakeLists edit needed - `jsi.cpp` is compiled into the static lib so library members that reference JSI symbols (e.g. `AudioFileProperties::CreateFromJSIValue`) link when a test first pulls them in; a static-lib member costs nothing unless demanded. If a new test triggers `Undefined symbols: facebook::jsi::...`, the referenced runtime source is missing from the lib — add it there rather than stubbing the symbol @@ -393,7 +395,7 @@ Resolution pitfalls learned the hard way (both handled inside `package-root.js`) | Flag | Android (CMake) | iOS (podspec) | Tests | |---|---|---|---| | `RN_AUDIO_API_FFMPEG_ENABLED` | `-DRN_AUDIO_API_FFMPEG_ENABLED=1` (only with `enableFFmpeg: true` in the app's package.json) | same | | Not set (off) | -| `RN_AUDIO_API_ENABLE_WORKLETS` | `-DRN_AUDIO_API_ENABLE_WORKLETS=1/0` | `-DRN_AUDIO_API_ENABLE_WORKLETS=1` | Always set to 0 | +| `RN_AUDIO_API_ENABLE_WORKLETS` | Not set (engine no longer reads it) | Not set | Not set. `wpt_tests/CMakeLists.txt` still passes `=0` | | `RCT_NEW_ARCH_ENABLED` | `-DRCT_NEW_ARCH_ENABLED` | `-DRCT_NEW_ARCH_ENABLED` | Not set | | `HAVE_ARM_NEON_INTRINSICS` | Set by CMake SIMD detection | Set by Xcode/Clang for arm64 | Set by CMake SIMD detection | | `HAVE_X86_SSE2` | Set by CMake SIMD detection | Not used | Set by CMake SIMD detection | diff --git a/.claude/skills/build-compilation-dependencies/build-details.md b/.claude/skills/build-compilation-dependencies/build-details.md index a0ab9b1c3..988c5a250 100644 --- a/.claude/skills/build-compilation-dependencies/build-details.md +++ b/.claude/skills/build-compilation-dependencies/build-details.md @@ -249,18 +249,25 @@ This means **tests build against the installed/published version of the library* ```cmake list(FILTER RNAUDIOAPI_SRC EXCLUDE REGEX ".*/audioapi/HostObjects/.*\\.cpp$") # no JSI in tests -list(FILTER RNAUDIOAPI_SRC EXCLUDE REGEX ".*/Worklet.*Node\\.cpp$") # worklets not compiled -list(REMOVE_ITEM RNAUDIOAPI_SRC ... "AudioContext.cpp") # needs real audio I/O +list(APPEND RNAUDIOAPI_SRC ".../HostObjects/utils/JsEnumParser.cpp") # the one exception +list(FILTER RNAUDIOAPI_SRC EXCLUDE REGEX ".*/AudioThreadGuard\\.cpp$") # compiled into each test executable +list(REMOVE_ITEM RNAUDIOAPI_SRC ... "AudioContext.cpp") # needs real audio I/O list(REMOVE_ITEM RNAUDIOAPI_SRC ... "FfmpegDecoder.cpp") # FFmpeg disabled in tests ``` +`tests_tsan` / `tests_asan` do not reuse those objects. `add_sanitized_tests` compiles a separate `rnaudioapi_` and `rnaudioapi_libs_` with the same `-fsanitize=` flags as the executable. Sanitizer instrumentation is per translation unit. + ### Compile definitions always set in tests ```cmake -add_compile_definitions(RN_AUDIO_API_ENABLE_WORKLETS=0) add_compile_definitions(RN_AUDIO_API_TEST=1) +add_compile_definitions(RN_AUDIO_API_STATIC_EXTERNAL_LIBS_DISABLED=1) +add_compile_definitions(MA_NO_LIBOPUS=1) +add_compile_definitions(MA_NO_LIBVORBIS=1) ``` +Directory-scoped, so the plain libraries, both sanitizer copies, and every test executable all receive them. `RN_AUDIO_API_ENABLE_WORKLETS` is not set; engine sources no longer read it. + Use `RN_AUDIO_API_TEST` in source code to conditionally compile test-only hooks. ### Google Test auto-fetched if not installed diff --git a/.claude/skills/turbo-modules/SKILL.md b/.claude/skills/turbo-modules/SKILL.md index 3c885d6cc..2899f929b 100644 --- a/.claude/skills/turbo-modules/SKILL.md +++ b/.claude/skills/turbo-modules/SKILL.md @@ -106,24 +106,13 @@ class AudioAPIModuleInstaller { static void injectJSIBindings( jsi::Runtime *jsiRuntime, const std::shared_ptr &jsCallInvoker, - const std::shared_ptr &audioEventHandlerRegistry, - std::shared_ptr uiRuntime = nullptr); + const std::shared_ptr &audioEventHandlerRegistry); }; ``` -Each `get*Function()` private method creates a `jsi::Function` via `jsi::Function::createFromHostFunction()`. The lambda captures the dependencies it needs (`jsCallInvoker`, `audioEventHandlerRegistry`, `uiRuntime`) by value (shared_ptr). +Each `get*Function()` private method creates a `jsi::Function` via `jsi::Function::createFromHostFunction()`. The lambda captures the dependencies it needs (`jsCallInvoker`, `audioEventHandlerRegistry`) by value (shared_ptr). The header is header-only; there is no `.cpp`. -**Worklets guard**: functions that need the worklets runtime (AudioContext, OfflineAudioContext) have an `#if RN_AUDIO_API_ENABLE_WORKLETS` branch: -```cpp -#if RN_AUDIO_API_ENABLE_WORKLETS - auto runtimeRegistry = RuntimeRegistry{.uiRuntime = uiRuntime}; - if (count > 1 && args[1].isObject()) { - runtimeRegistry.audioRuntime = worklets::extractWorkletRuntime(runtime, args[1]); - } -#else - auto runtimeRegistry = RuntimeRegistry{}; -#endif -``` +`RN_AUDIO_API_ENABLE_WORKLETS` is gone. `createAudioContext` takes sample rate, an optional latency-hint string, and an optional Android output-profile string. It does not take a worklet runtime. **Adding a new top-level global**: add a `static jsi::Function getCreateXxxFunction(...)` private method and a `setProperty("createXxx", ...)` call in `injectJSIBindings`. This is only needed for objects that JS creates directly (not objects created as properties of another HostObject). diff --git a/packages/react-native-audio-api/common/cpp/test/CMakeLists.txt b/packages/react-native-audio-api/common/cpp/test/CMakeLists.txt index f9a76bd33..7da0cc6c6 100644 --- a/packages/react-native-audio-api/common/cpp/test/CMakeLists.txt +++ b/packages/react-native-audio-api/common/cpp/test/CMakeLists.txt @@ -71,28 +71,40 @@ list(REMOVE_ITEM RNAUDIOAPI_LIBS "${REACT_NATIVE_AUDIO_API_DIR}/common/cpp/audioapi/libs/miniaudio/decoders/libvorbis/miniaudio_libvorbis.c" ) -add_library(rnaudioapi STATIC ${RNAUDIOAPI_SRC} "${JSI_DIR}/jsi/jsi.cpp") -add_library(rnaudioapi_libs STATIC ${RNAUDIOAPI_LIBS}) - -target_include_directories(rnaudioapi PUBLIC - ${ROOT}/packages/react-native-audio-api/common/cpp - ${JSI_DIR} - "${REACT_NATIVE_DIR}/ReactCommon" - "${REACT_NATIVE_DIR}/ReactCommon/callinvoker" -) +# Defines the static libraries rnaudioapi${suffix} (engine) and +# rnaudioapi_libs${suffix} (third-party C sources), both compiled with the +# extra compile options passed after `suffix`. +function(add_rnaudioapi_libraries suffix) + set(engine rnaudioapi${suffix}) + set(third_party rnaudioapi_libs${suffix}) + + add_library(${engine} STATIC ${RNAUDIOAPI_SRC} "${JSI_DIR}/jsi/jsi.cpp") + add_library(${third_party} STATIC ${RNAUDIOAPI_LIBS}) + target_compile_options(${engine} PRIVATE ${ARGN}) + target_compile_options(${third_party} PRIVATE ${ARGN}) + + target_include_directories(${engine} PUBLIC + ${ROOT}/packages/react-native-audio-api/common/cpp + ${JSI_DIR} + "${REACT_NATIVE_DIR}/ReactCommon" + "${REACT_NATIVE_DIR}/ReactCommon/callinvoker" + ) -if(GTest_FOUND) - target_link_libraries(rnaudioapi PUBLIC GTest::gtest GTest::gmock) -else() - target_include_directories(rnaudioapi PUBLIC - ${gtest_SOURCE_DIR}/include - ${gmock_SOURCE_DIR}/include + if(GTest_FOUND) + target_link_libraries(${engine} PUBLIC GTest::gtest GTest::gmock) + else() + target_include_directories(${engine} PUBLIC + ${gtest_SOURCE_DIR}/include + ${gmock_SOURCE_DIR}/include + ) + endif() + + target_include_directories(${third_party} PUBLIC + ${ROOT}/packages/react-native-audio-api/common/cpp ) -endif() +endfunction() -target_include_directories(rnaudioapi_libs PUBLIC - ${ROOT}/packages/react-native-audio-api/common/cpp -) +add_rnaudioapi_libraries("") file(GLOB_RECURSE test_src CONFIGURE_DEPENDS @@ -137,18 +149,22 @@ include(GoogleTest) gtest_discover_tests(tests DISCOVERY_MODE PRE_TEST) if(NOT ENABLE_COVERAGE) - # --- Thread Sanitizer build --- - add_executable(tests_tsan ${test_src} ${AUDIO_THREAD_GUARD_SRC}) - target_compile_options(tests_tsan PRIVATE -fsanitize=thread -g -O1) - target_link_options(tests_tsan PRIVATE -fsanitize=thread) - target_link_libraries(tests_tsan rnaudioapi rnaudioapi_libs GTest::gtest_main GTest::gmock) - - # --- Address + Undefined Behavior Sanitizer build --- - add_executable(tests_asan ${test_src} ${AUDIO_THREAD_GUARD_SRC}) - target_compile_options(tests_asan PRIVATE -fsanitize=address,undefined -g -O1) - target_link_options(tests_asan PRIVATE -fsanitize=address,undefined) - target_link_libraries(tests_asan rnaudioapi rnaudioapi_libs GTest::gtest_main GTest::gmock) - - gtest_discover_tests(tests_tsan DISCOVERY_MODE PRE_TEST) - gtest_discover_tests(tests_asan DISCOVERY_MODE PRE_TEST) + function(add_sanitized_tests name compile_flags link_flags) + add_rnaudioapi_libraries(_${name} ${compile_flags}) + add_executable(tests_${name} ${test_src} ${AUDIO_THREAD_GUARD_SRC}) + target_compile_options(tests_${name} PRIVATE ${compile_flags}) + target_link_options(tests_${name} PRIVATE ${link_flags}) + target_link_libraries(tests_${name} + rnaudioapi_${name} + rnaudioapi_libs_${name} + GTest::gtest_main + GTest::gmock) + gtest_discover_tests(tests_${name} DISCOVERY_MODE PRE_TEST) + endfunction() + + add_sanitized_tests(tsan "-fsanitize=thread;-g;-O1" "-fsanitize=thread") + add_sanitized_tests( + asan + "-fsanitize=address,undefined;-g;-O1" + "-fsanitize=address,undefined") endif()