Fix: prevent race condition crash in audio playback using pthread mutexes

Root cause: ALSA assertion failure `snd_pcm_writei: Assertion 'pcm' failed`
when pcm_playback_handle became NULL during concurrent write operations.

The crash occurred because:
1. Thread A checks pcm_playback_handle != NULL (passes)
2. Thread B calls jetkvm_audio_playback_close(), sets handle = NULL
3. Thread A calls snd_pcm_writei(NULL, ...) → SIGABRT

Solution: Added pthread mutexes to protect concurrent access:
- playback_mutex protects pcm_playback_handle in decode_write and close
- capture_mutex protects pcm_capture_handle in read_encode and close

All critical sections now acquire mutex before accessing ALSA handles,
preventing the NULL pointer from being passed to ALSA functions.
This commit is contained in:
Alex P 2025-11-18 00:30:01 +02:00
parent 3f141c8e9d
commit 94cab8b2ac
1 changed files with 60 additions and 4 deletions

View File

@ -24,6 +24,7 @@
#include <sched.h> #include <sched.h>
#include <time.h> #include <time.h>
#include <signal.h> #include <signal.h>
#include <pthread.h>
// ARM NEON SIMD support (always available on JetKVM's ARM Cortex-A7) // ARM NEON SIMD support (always available on JetKVM's ARM Cortex-A7)
#include <arm_neon.h> #include <arm_neon.h>
@ -77,6 +78,10 @@ static uint32_t max_backoff_us_global = 500000;
static volatile int capture_stop_requested = 0; static volatile int capture_stop_requested = 0;
static volatile int playback_stop_requested = 0; static volatile int playback_stop_requested = 0;
// Mutexes to protect concurrent access to ALSA handles during close
static pthread_mutex_t capture_mutex = PTHREAD_MUTEX_INITIALIZER;
static pthread_mutex_t playback_mutex = PTHREAD_MUTEX_INITIALIZER;
int jetkvm_audio_capture_init(); int jetkvm_audio_capture_init();
void jetkvm_audio_capture_close(); void jetkvm_audio_capture_close();
int jetkvm_audio_read_encode(void *opus_buf); int jetkvm_audio_read_encode(void *opus_buf);
@ -452,14 +457,19 @@ __attribute__((hot)) int jetkvm_audio_read_encode(void * __restrict__ opus_buf)
SIMD_PREFETCH(pcm_buffer, 0, 0); SIMD_PREFETCH(pcm_buffer, 0, 0);
SIMD_PREFETCH(pcm_buffer + 64, 0, 1); SIMD_PREFETCH(pcm_buffer + 64, 0, 1);
// Acquire mutex to protect against concurrent close
pthread_mutex_lock(&capture_mutex);
if (__builtin_expect(!capture_initialized || !pcm_capture_handle || !encoder || !opus_buf, 0)) { if (__builtin_expect(!capture_initialized || !pcm_capture_handle || !encoder || !opus_buf, 0)) {
TRACE_LOG("[AUDIO_OUTPUT] jetkvm_audio_read_encode: Failed safety checks - capture_initialized=%d, pcm_capture_handle=%p, encoder=%p, opus_buf=%p\n", TRACE_LOG("[AUDIO_OUTPUT] jetkvm_audio_read_encode: Failed safety checks - capture_initialized=%d, pcm_capture_handle=%p, encoder=%p, opus_buf=%p\n",
capture_initialized, pcm_capture_handle, encoder, opus_buf); capture_initialized, pcm_capture_handle, encoder, opus_buf);
pthread_mutex_unlock(&capture_mutex);
return -1; return -1;
} }
retry_read: retry_read:
if (__builtin_expect(capture_stop_requested, 0)) { if (__builtin_expect(capture_stop_requested, 0)) {
pthread_mutex_unlock(&capture_mutex);
return -1; return -1;
} }
@ -469,13 +479,17 @@ retry_read:
if (pcm_rc == -EPIPE) { if (pcm_rc == -EPIPE) {
recovery_attempts++; recovery_attempts++;
if (recovery_attempts > max_recovery_attempts) { if (recovery_attempts > max_recovery_attempts) {
pthread_mutex_unlock(&capture_mutex);
return -1; return -1;
} }
err = snd_pcm_prepare(pcm_capture_handle); err = snd_pcm_prepare(pcm_capture_handle);
if (err < 0) { if (err < 0) {
snd_pcm_drop(pcm_capture_handle); snd_pcm_drop(pcm_capture_handle);
err = snd_pcm_prepare(pcm_capture_handle); err = snd_pcm_prepare(pcm_capture_handle);
if (err < 0) return -1; if (err < 0) {
pthread_mutex_unlock(&capture_mutex);
return -1;
}
} }
goto retry_read; goto retry_read;
} else if (pcm_rc == -EAGAIN) { } else if (pcm_rc == -EAGAIN) {
@ -484,20 +498,29 @@ retry_read:
} else if (pcm_rc == -ESTRPIPE) { } else if (pcm_rc == -ESTRPIPE) {
recovery_attempts++; recovery_attempts++;
if (recovery_attempts > max_recovery_attempts) { if (recovery_attempts > max_recovery_attempts) {
pthread_mutex_unlock(&capture_mutex);
return -1; return -1;
} }
uint8_t resume_attempts = 0; uint8_t resume_attempts = 0;
while ((err = snd_pcm_resume(pcm_capture_handle)) == -EAGAIN && resume_attempts < 10) { while ((err = snd_pcm_resume(pcm_capture_handle)) == -EAGAIN && resume_attempts < 10) {
if (capture_stop_requested) return -1; if (capture_stop_requested) {
pthread_mutex_unlock(&capture_mutex);
return -1;
}
snd_pcm_wait(pcm_capture_handle, sleep_milliseconds); snd_pcm_wait(pcm_capture_handle, sleep_milliseconds);
resume_attempts++; resume_attempts++;
} }
if (err < 0) { if (err < 0) {
err = snd_pcm_prepare(pcm_capture_handle); err = snd_pcm_prepare(pcm_capture_handle);
if (err < 0) return -1; if (err < 0) {
pthread_mutex_unlock(&capture_mutex);
return -1;
}
} }
pthread_mutex_unlock(&capture_mutex);
return 0; return 0;
} else if (pcm_rc == -ENODEV) { } else if (pcm_rc == -ENODEV) {
pthread_mutex_unlock(&capture_mutex);
return -1; return -1;
} else if (pcm_rc == -EIO) { } else if (pcm_rc == -EIO) {
recovery_attempts++; recovery_attempts++;
@ -508,6 +531,7 @@ retry_read:
goto retry_read; goto retry_read;
} }
} }
pthread_mutex_unlock(&capture_mutex);
return -1; return -1;
} else { } else {
recovery_attempts++; recovery_attempts++;
@ -517,6 +541,7 @@ retry_read:
snd_pcm_wait(pcm_capture_handle, 1); // Wait 1ms for device snd_pcm_wait(pcm_capture_handle, 1); // Wait 1ms for device
goto retry_read; goto retry_read;
} }
pthread_mutex_unlock(&capture_mutex);
return -1; return -1;
} }
} }
@ -528,6 +553,7 @@ retry_read:
} }
nb_bytes = opus_encode(encoder, pcm_buffer, frame_size, out, max_packet_size); nb_bytes = opus_encode(encoder, pcm_buffer, frame_size, out, max_packet_size);
pthread_mutex_unlock(&capture_mutex);
return nb_bytes; return nb_bytes;
} }
@ -621,14 +647,19 @@ __attribute__((hot)) int jetkvm_audio_decode_write(void * __restrict__ opus_buf,
SIMD_PREFETCH(in, 0, 0); SIMD_PREFETCH(in, 0, 0);
// Acquire mutex to protect against concurrent close
pthread_mutex_lock(&playback_mutex);
if (__builtin_expect(!playback_initialized || !pcm_playback_handle || !decoder || !opus_buf || opus_size <= 0, 0)) { if (__builtin_expect(!playback_initialized || !pcm_playback_handle || !decoder || !opus_buf || opus_size <= 0, 0)) {
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Failed safety checks - playback_initialized=%d, pcm_playback_handle=%p, decoder=%p, opus_buf=%p, opus_size=%d\n", TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Failed safety checks - playback_initialized=%d, pcm_playback_handle=%p, decoder=%p, opus_buf=%p, opus_size=%d\n",
playback_initialized, pcm_playback_handle, decoder, opus_buf, opus_size); playback_initialized, pcm_playback_handle, decoder, opus_buf, opus_size);
pthread_mutex_unlock(&playback_mutex);
return -1; return -1;
} }
if (opus_size > max_packet_size) { if (opus_size > max_packet_size) {
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Opus packet too large - size=%d, max=%d\n", opus_size, max_packet_size); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Opus packet too large - size=%d, max=%d\n", opus_size, max_packet_size);
pthread_mutex_unlock(&playback_mutex);
return -1; return -1;
} }
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Processing Opus packet - size=%d bytes\n", opus_size); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Processing Opus packet - size=%d bytes\n", opus_size);
@ -645,6 +676,7 @@ __attribute__((hot)) int jetkvm_audio_decode_write(void * __restrict__ opus_buf,
pcm_frames = opus_decode(decoder, NULL, 0, pcm_buffer, frame_size, 1); pcm_frames = opus_decode(decoder, NULL, 0, pcm_buffer, frame_size, 1);
if (pcm_frames < 0) { if (pcm_frames < 0) {
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Packet loss concealment also failed with error %d\n", pcm_frames); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Packet loss concealment also failed with error %d\n", pcm_frames);
pthread_mutex_unlock(&playback_mutex);
return -1; return -1;
} }
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Packet loss concealment succeeded, recovered %d frames\n", pcm_frames); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Packet loss concealment succeeded, recovered %d frames\n", pcm_frames);
@ -653,6 +685,7 @@ __attribute__((hot)) int jetkvm_audio_decode_write(void * __restrict__ opus_buf,
retry_write: retry_write:
if (__builtin_expect(playback_stop_requested, 0)) { if (__builtin_expect(playback_stop_requested, 0)) {
pthread_mutex_unlock(&playback_mutex);
return -1; return -1;
} }
@ -665,6 +698,7 @@ retry_write:
recovery_attempts++; recovery_attempts++;
if (recovery_attempts > max_recovery_attempts) { if (recovery_attempts > max_recovery_attempts) {
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Buffer underrun recovery failed after %d attempts\n", max_recovery_attempts); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Buffer underrun recovery failed after %d attempts\n", max_recovery_attempts);
pthread_mutex_unlock(&playback_mutex);
return -2; return -2;
} }
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Buffer underrun detected, attempting recovery (attempt %d)\n", recovery_attempts); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Buffer underrun detected, attempting recovery (attempt %d)\n", recovery_attempts);
@ -675,6 +709,7 @@ retry_write:
err = snd_pcm_prepare(pcm_playback_handle); err = snd_pcm_prepare(pcm_playback_handle);
if (err < 0) { if (err < 0) {
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: drop+prepare recovery failed (%s)\n", snd_strerror(err)); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: drop+prepare recovery failed (%s)\n", snd_strerror(err));
pthread_mutex_unlock(&playback_mutex);
return -2; return -2;
} }
} }
@ -684,12 +719,16 @@ retry_write:
recovery_attempts++; recovery_attempts++;
if (recovery_attempts > max_recovery_attempts) { if (recovery_attempts > max_recovery_attempts) {
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Device suspend recovery failed after %d attempts\n", max_recovery_attempts); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Device suspend recovery failed after %d attempts\n", max_recovery_attempts);
pthread_mutex_unlock(&playback_mutex);
return -2; return -2;
} }
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Device suspended, attempting resume (attempt %d)\n", recovery_attempts); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Device suspended, attempting resume (attempt %d)\n", recovery_attempts);
uint8_t resume_attempts = 0; uint8_t resume_attempts = 0;
while ((err = snd_pcm_resume(pcm_playback_handle)) == -EAGAIN && resume_attempts < 10) { while ((err = snd_pcm_resume(pcm_playback_handle)) == -EAGAIN && resume_attempts < 10) {
if (playback_stop_requested) return -1; if (playback_stop_requested) {
pthread_mutex_unlock(&playback_mutex);
return -1;
}
snd_pcm_wait(pcm_playback_handle, sleep_milliseconds); snd_pcm_wait(pcm_playback_handle, sleep_milliseconds);
resume_attempts++; resume_attempts++;
} }
@ -698,13 +737,16 @@ retry_write:
err = snd_pcm_prepare(pcm_playback_handle); err = snd_pcm_prepare(pcm_playback_handle);
if (err < 0) { if (err < 0) {
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Prepare fallback failed (%s)\n", snd_strerror(err)); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Prepare fallback failed (%s)\n", snd_strerror(err));
pthread_mutex_unlock(&playback_mutex);
return -2; return -2;
} }
} }
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Device suspend recovery successful, skipping frame\n"); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Device suspend recovery successful, skipping frame\n");
pthread_mutex_unlock(&playback_mutex);
return 0; return 0;
} else if (pcm_rc == -ENODEV) { } else if (pcm_rc == -ENODEV) {
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Device disconnected (ENODEV) - critical error\n"); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Device disconnected (ENODEV) - critical error\n");
pthread_mutex_unlock(&playback_mutex);
return -2; return -2;
} else if (pcm_rc == -EIO) { } else if (pcm_rc == -EIO) {
recovery_attempts++; recovery_attempts++;
@ -718,6 +760,7 @@ retry_write:
} }
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: I/O error recovery failed (%s)\n", snd_strerror(err)); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: I/O error recovery failed (%s)\n", snd_strerror(err));
} }
pthread_mutex_unlock(&playback_mutex);
return -2; return -2;
} else if (pcm_rc == -EAGAIN) { } else if (pcm_rc == -EAGAIN) {
recovery_attempts++; recovery_attempts++;
@ -727,6 +770,7 @@ retry_write:
goto retry_write; goto retry_write;
} }
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Device not ready recovery failed after %d attempts\n", max_recovery_attempts); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Device not ready recovery failed after %d attempts\n", max_recovery_attempts);
pthread_mutex_unlock(&playback_mutex);
return -2; return -2;
} else { } else {
recovery_attempts++; recovery_attempts++;
@ -736,10 +780,12 @@ retry_write:
goto retry_write; goto retry_write;
} }
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Unrecoverable error %d (%s)\n", pcm_rc, snd_strerror(pcm_rc)); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Unrecoverable error %d (%s)\n", pcm_rc, snd_strerror(pcm_rc));
pthread_mutex_unlock(&playback_mutex);
return -2; return -2;
} }
} }
TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Successfully wrote %d PCM frames to device\n", pcm_frames); TRACE_LOG("[AUDIO_INPUT] jetkvm_audio_decode_write: Successfully wrote %d PCM frames to device\n", pcm_frames);
pthread_mutex_unlock(&playback_mutex);
return pcm_frames; return pcm_frames;
} }
@ -758,6 +804,9 @@ void jetkvm_audio_playback_close() {
return; return;
} }
// Acquire mutex to prevent concurrent write operations
pthread_mutex_lock(&playback_mutex);
if (decoder) { if (decoder) {
opus_decoder_destroy(decoder); opus_decoder_destroy(decoder);
decoder = NULL; decoder = NULL;
@ -768,6 +817,8 @@ void jetkvm_audio_playback_close() {
pcm_playback_handle = NULL; pcm_playback_handle = NULL;
} }
pthread_mutex_unlock(&playback_mutex);
playback_stop_requested = 0; playback_stop_requested = 0;
} }
@ -784,6 +835,9 @@ void jetkvm_audio_capture_close() {
return; return;
} }
// Acquire mutex to prevent concurrent read operations
pthread_mutex_lock(&capture_mutex);
if (pcm_capture_handle) { if (pcm_capture_handle) {
snd_pcm_drop(pcm_capture_handle); snd_pcm_drop(pcm_capture_handle);
snd_pcm_close(pcm_capture_handle); snd_pcm_close(pcm_capture_handle);
@ -794,5 +848,7 @@ void jetkvm_audio_capture_close() {
encoder = NULL; encoder = NULL;
} }
pthread_mutex_unlock(&capture_mutex);
capture_stop_requested = 0; capture_stop_requested = 0;
} }