ConvolverNode: SIGSEGV in Convolver::process caused by a race in ThreadPool::wait()
Los mantenedores suelen responder en 2 días
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 4/5
- Tiempo estimado
- 3-5 días
- Aptitud para principiantes
- 48/100
- Tipo de issue
- Error
- Claridad
- Bien especificado
- Estado de actividad
- Activo
- Stack tecnológico
- android, cpp, react-native
- Área
- audio-video-rtc
Línea de trabajo
Start with common/cpp/audioapi/utils/ThreadPool.hpp, focusing on schedule() and wait(), then trace the captured processingBuffer references in ConvolverNode::processNode. Reproduce with the issue’s standalone harness and widened scheduling window. Done means wait() does not return before all tasks in the current batch complete, with a regression test covering the race and no dangling buffer access.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
Description
We got a native crash in production (Android, arm64) inside Convolver::process:
#00 libc.so (__memcpy_aarch64_simd+232)
#01 libreact-native-audio-api.so (audioapi::Convolver::process(audioapi::AlignedAudioArray<64ul> const&, audioapi::AlignedAudioArray<64ul>&)+112)
#02 libreact-native-audio-api.so (audioapi::move_only_function<void ()>::callable_impl<... audioapi::ThreadPool::schedule<audioapi::ConvolverNode::performConvolution(std::shared_ptr<audioapi::AlignedAudioBuffer<64ul>> const&)::$_1 ...>::operator()())
#03 libreact-native-audio-api.so (audioapi::ThreadPool::workerThreadFunc(...)+220)
#04 libreact-native-audio-api.so (std::__thread_proxy ...)
#05 libc.so (__pthread_start)
It has happened once so far. I think it comes from a race in ThreadPool::wait() (common/cpp/audioapi/utils/ThreadPool.hpp). The code is the same in 0.13.2, 0.13.6 and on main as of today.
The interleaving:
- Quantum N: the audio thread schedules the convolver tasks. All of them finish before the audio thread reaches
wait(). The last worker has donetasksScheduled.fetch_sub(count is now 0), but it gets preempted beforewaitingForTasks.store(false)/notify_one(). - The audio thread calls
wait(): it storestrue, seestasksScheduled == 0, storesfalseand returns. That is correct for quantum N. - Quantum N+1: the audio thread schedules new tasks (count > 0), calls
wait(), storestrueand blocks inwaitingForTasks.wait(true). - The worker from step 1 resumes and runs
store(false)+notify_one().wait()returns while the tasks from quantum N+1 are still pending or running.
After that, ConvolverNode::processNode returns early. The scheduled lambdas capture processingBuffer by reference ([&, i] in the mono branch, &processingBuffer in the stereo branch). That is a const std::shared_ptr<DSPAudioBuffer>& bound to a local in AudioNode::processAudio, so once the frame is gone the worker reads a dangling reference. Convolver::process then copies from it (_inputBuffer->copy(input, ...)), which matches the memcpy frame at process+112.
Steps to reproduce
The race window is a few instructions wide, so it does not show up in normal use. I reproduced it with a standalone harness that compiles ThreadPool.hpp as is, on macOS with clang++ -std=c++20.
- Stock header: with fast tasks, 2,000,000 quanta give 0 early returns. The window is too small to hit by chance.
- Widened window: I put a 2.5 ms sleep between
fetch_subreaching 0 andstore(false)on 1 in 7 tasks. I also delay the scheduling thread for 3 ms beforewait()on every third quantum, which simulates the audio thread being preempted. The sleep only stands in for a preempted worker and does not change the logic. In this setup,wait()returned with 0/2 tasks done in 142 of 3,000 quanta. - With the fix below: the same widened setup gives 0 early returns.
Harness (the check after wait() is the only assertion):
#include <audioapi/utils/ThreadPool.hpp>
#include <atomic>
#include <chrono>
#include <cstdio>
#include <thread>
int main() {
audioapi::ThreadPool pool(4);
std::atomic<int> done{0};
long early = 0;
for (long q = 0; q < 3000; ++q) {
done.store(0);
for (int i = 0; i < 2; ++i)
pool.schedule([&done] {
std::this_thread::sleep_for(std::chrono::microseconds(1000));
done.fetch_add(1);
});
if (q % 3 == 0) std::this_thread::sleep_for(std::chrono::microseconds(3000)); // scheduler preempted
pool.wait();
if (done.load() != 2) { ++early; while (done.load() != 2) {} }
}
std::printf("early_returns=%ld\n", early);
}
For the widened run, add this inside the bound task's if (left == 0) branch, before store(false):
if (counter++ % 7 == 0) std::this_thread::sleep_for(std::chrono::microseconds(2500));
Suggested fix: wait on the counter itself, so a late wake-up from a previous batch can't release the waiter:
// in schedule()'s bound task, after f(...)
if (cntrl->tasksScheduled.fetch_sub(1, std::memory_order_acq_rel) == 1) {
cntrl->tasksScheduled.notify_all();
}
void wait() {
size_t pending;
while ((pending = controlBlock_->tasksScheduled.load(std::memory_order_acquire)) != 0) {
controlBlock_->tasksScheduled.wait(pending, std::memory_order_acquire);
}
}
With this, waitingForTasks is no longer needed. As a second safeguard, performConvolution could capture processingBuffer by value (it's a shared_ptr), so a task that outlives processNode doesn't read a dead stack slot.
Snack or a link to a repository
No public repo. The harness above is self-contained and needs only ThreadPool.hpp, SpscChannel.hpp, MoveOnlyFunction.hpp and core/utils/Constants.h on the include path.
React Native Audio API version
0.13.2 (code is unchanged in 0.13.6 and on main)
React Native version
0.86.2 (Expo SDK 57)
Platforms
Android
Architecture
New Architecture (bridgeless)
Build type
Release
Device
Real device
Device model
Android 16 (API 36), arm64
Acknowledgements
Yes, I searched the existing issues first.
- Lenguaje dominante
- C++
- Estrellas
- 848
- Forks
- 95
- Merge medio
- 4 d 20 min
- PR fusionados (30 d)
- 31
Preparar el entorno
- Sin Dockerfile ni archivo de Docker Compose
- Tiene una plantilla de pull request
- Leer la guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de software-mansion/react-native-audio-api
-
android bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
software-mansion/react-native-audio-api#1273 · 5 comentarios ·
Los mantenedores suelen responder en 2 días
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
software-mansion/react-native-audio-api#1238 ·
Los mantenedores suelen responder en 2 días
-
Dificultad 4/5 3-5 días Aptitud para principiantes 38/100
software-mansion/react-native-audio-api#1340 · 1 comentario ·
Los mantenedores suelen responder en 2 días
-
Channel negotiation drops buffer swaps when several graph changes land in one render quantumPosiblemente ocupada Un pull request vinculado a esta issue está abierto o ya se fusionó. Abiertobug
Dificultad 4/5 3-5 días Aptitud para principiantes 48/100
software-mansion/react-native-audio-api#1332 · 4 comentarios ·
Los mantenedores suelen responder en 2 días
-
bug ios pkg:audio-api
Dificultad 4/5 3-5 días Aptitud para principiantes 25/100
software-mansion/react-native-audio-api#1298 · 4 comentarios ·
Los mantenedores suelen responder en 2 días
Todos los issues de software-mansion/react-native-audio-api
Issues similares
-
[Cleanup] Include the public header in work_split.cppPosiblemente ocupada @blozano-tt la tomó hoy. Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
tenstorrent/tt-metal#60850 ·
Los mantenedores suelen responder en 1 día
-
bug core p3
Dificultad 2/5 1-3 horas Aptitud para principiantes 70/100
RunanywhereAI/runanywhere-sdks#1083 ·
Los mantenedores suelen responder en 5 días
-
Dificultad 1/5 Menos de una hora Aptitud para principiantes 75/100
tesseract-ocr/tesseract#4653 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
-
comp-datalake performance
Dificultad 2/5 1-3 horas Aptitud para principiantes 85/100
ClickHouse/ClickHouse#126044 ·
Los mantenedores suelen responder en 1 día