From b35a6b7d9265a0e16e1c57e11ab03a5b6c6b9cc8 Mon Sep 17 00:00:00 2001 From: Siavash Sameni Date: Thu, 9 Apr 2026 19:11:16 +0400 Subject: [PATCH] fix(wzp-native): copy WzpOboeRings by value, not by pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PlayoutCallback::onAudioReady crashed with SIGSEGV(SEGV_ACCERR) on the first AAudio callback because g_rings was a `const WzpOboeRings*` pointing at the caller's stack frame. wzp_native_audio_start() constructs the rings struct as a stack local in Rust, passes &rings to wzp_oboe_start (which stored the raw pointer), and returns — at which point the stack frame unwinds and g_rings becomes a dangling reference. The first audio callback then read from freed memory and died. - g_rings is now a static WzpOboeRings value (was `const WzpOboeRings*`). The raw int16 buffer + atomic index pointers inside the struct still point into the Rust-owned AudioBackend singleton, which is leaked for the lifetime of the process, so deep-copying the struct by value is safe and keeps the inner pointers valid forever. - g_rings_valid atomic bool gates the audio-callback reads: set to true after the value copy in wzp_oboe_start, cleared in wzp_oboe_stop BEFORE the streams are torn down so any in-flight callback sees "no backend" and returns Stop instead of racing on g_rings. - All g_rings->x accesses in the capture + playout callbacks switched to g_rings.x (member-of-value). Reproduced on Pixel 6 / Android 15 with build 0105b0f: F libc: Fatal signal 11 (SIGSEGV), code 2 (SEGV_ACCERR), fault addr 0x71aa717eb0 in tid 11822 (AudioTrack) #00 PlayoutCallback::onAudioReady(oboe::AudioStream*, void*, int)+120 #01 oboe::AudioStream::fireDataCallback(void*, int)+136 ... --- crates/wzp-native/cpp/oboe_bridge.cpp | 47 ++++++++++++++++++--------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/crates/wzp-native/cpp/oboe_bridge.cpp b/crates/wzp-native/cpp/oboe_bridge.cpp index 066bb61..d55e066 100644 --- a/crates/wzp-native/cpp/oboe_bridge.cpp +++ b/crates/wzp-native/cpp/oboe_bridge.cpp @@ -64,7 +64,15 @@ static inline void ring_read(int16_t* buf, int32_t capacity, static std::shared_ptr g_capture_stream; static std::shared_ptr g_playout_stream; -static const WzpOboeRings* g_rings = nullptr; +// Value copy — the WzpOboeRings the Rust side passes us lives on the caller's +// stack frame and goes away as soon as wzp_oboe_start returns. The raw +// int16/atomic pointers INSIDE the struct point into the Rust-owned, leaked- +// for-the-lifetime-of-the-process AudioBackend singleton, so copying the +// struct by value is safe and keeps the inner pointers valid indefinitely. +// g_rings_valid guards the audio-callback-side read; clearing it in stop() +// signals "no backend" to the callbacks which then return silence + Stop. +static WzpOboeRings g_rings{}; +static std::atomic g_rings_valid{false}; static std::atomic g_running{false}; static std::atomic g_capture_latency_ms{0.0f}; static std::atomic g_playout_latency_ms{0.0f}; @@ -79,18 +87,19 @@ public: oboe::AudioStream* stream, void* audioData, int32_t numFrames) override { - if (!g_running.load(std::memory_order_relaxed) || !g_rings) { + if (!g_running.load(std::memory_order_relaxed) || + !g_rings_valid.load(std::memory_order_acquire)) { return oboe::DataCallbackResult::Stop; } const int16_t* src = static_cast(audioData); - int32_t avail = ring_available_write(g_rings->capture_write_idx, - g_rings->capture_read_idx, - g_rings->capture_capacity); + int32_t avail = ring_available_write(g_rings.capture_write_idx, + g_rings.capture_read_idx, + g_rings.capture_capacity); int32_t to_write = (numFrames < avail) ? numFrames : avail; if (to_write > 0) { - ring_write(g_rings->capture_buf, g_rings->capture_capacity, - g_rings->capture_write_idx, g_rings->capture_read_idx, + ring_write(g_rings.capture_buf, g_rings.capture_capacity, + g_rings.capture_write_idx, g_rings.capture_read_idx, src, to_write); } @@ -115,20 +124,21 @@ public: oboe::AudioStream* stream, void* audioData, int32_t numFrames) override { - if (!g_running.load(std::memory_order_relaxed) || !g_rings) { + if (!g_running.load(std::memory_order_relaxed) || + !g_rings_valid.load(std::memory_order_acquire)) { memset(audioData, 0, numFrames * sizeof(int16_t)); return oboe::DataCallbackResult::Stop; } int16_t* dst = static_cast(audioData); - int32_t avail = ring_available_read(g_rings->playout_write_idx, - g_rings->playout_read_idx, - g_rings->playout_capacity); + int32_t avail = ring_available_read(g_rings.playout_write_idx, + g_rings.playout_read_idx, + g_rings.playout_capacity); int32_t to_read = (numFrames < avail) ? numFrames : avail; if (to_read > 0) { - ring_read(g_rings->playout_buf, g_rings->playout_capacity, - g_rings->playout_write_idx, g_rings->playout_read_idx, + ring_read(g_rings.playout_buf, g_rings.playout_capacity, + g_rings.playout_write_idx, g_rings.playout_read_idx, dst, to_read); } // Fill remainder with silence on underrun @@ -160,7 +170,11 @@ int wzp_oboe_start(const WzpOboeConfig* config, const WzpOboeRings* rings) { return -1; } - g_rings = rings; + // Deep-copy the rings struct into static storage BEFORE we publish it to + // the audio callbacks — `rings` points at the caller's stack frame and + // goes away as soon as this function returns. + g_rings = *rings; + g_rings_valid.store(true, std::memory_order_release); // Build capture stream oboe::AudioStreamBuilder captureBuilder; @@ -233,6 +247,10 @@ int wzp_oboe_start(const WzpOboeConfig* config, const WzpOboeRings* rings) { void wzp_oboe_stop(void) { g_running.store(false, std::memory_order_release); + // Tell the audio callbacks to stop touching g_rings BEFORE we tear down + // the streams, so any in-flight callback returns Stop instead of reading + // stale pointers. + g_rings_valid.store(false, std::memory_order_release); if (g_capture_stream) { g_capture_stream->requestStop(); @@ -245,7 +263,6 @@ void wzp_oboe_stop(void) { g_playout_stream.reset(); } - g_rings = nullptr; LOGI("Oboe stopped"); }