fix: make undo history fields atomic and protect clip buffer access in save/load thread

Co-authored-by: aider (deepseek/deepseek-coder) <aider@aider.chat>
This commit is contained in:
Loic Coenen
2026-05-02 11:19:33 +00:00
parent fde1a5cb04
commit b6cea54a89
2 changed files with 47 additions and 30 deletions

View File

@@ -317,12 +317,22 @@ void* save_load_thread_func(void *arg) {
// Build filename: samples/clip_<index>.wav // Build filename: samples/clip_<index>.wav
snprintf(filepath, sizeof(filepath), "samples/clip_%d.wav", req.clip_index); snprintf(filepath, sizeof(filepath), "samples/clip_%d.wav", req.clip_index);
if (clip->buffer && clip->buffer_size > 0) { // Atomically read buffer size and copy buffer to avoid race with audio thread
int result = save_wav_float(filepath, clip->buffer, clip->buffer_size, engine->sample_rate); size_t buf_size = (size_t)atomic_load(&clip->buffer_size);
if (result == 0) { if (clip->buffer && buf_size > 0) {
printf("Saved clip %d to %s (%zu samples)\n", req.clip_index, filepath, clip->buffer_size); // Copy buffer data to avoid race with audio thread writing to it
float *buffer_copy = (float *)malloc(buf_size * sizeof(float));
if (buffer_copy) {
memcpy(buffer_copy, clip->buffer, buf_size * sizeof(float));
int result = save_wav_float(filepath, buffer_copy, buf_size, engine->sample_rate);
free(buffer_copy);
if (result == 0) {
printf("Saved clip %d to %s (%zu samples)\n", req.clip_index, filepath, buf_size);
} else {
fprintf(stderr, "Failed to save clip %d to %s\n", req.clip_index, filepath);
}
} else { } else {
fprintf(stderr, "Failed to save clip %d to %s\n", req.clip_index, filepath); fprintf(stderr, "Failed to allocate buffer copy for clip %d\n", req.clip_index);
} }
} }
break; break;
@@ -353,8 +363,8 @@ void* save_load_thread_func(void *arg) {
float *old_buffer = atomic_exchange(&clip->buffer, clip_buffer); float *old_buffer = atomic_exchange(&clip->buffer, clip_buffer);
// Update clip state atomically // Update clip state atomically
clip->state = CLIP_LOOPING; atomic_store(&clip->state, CLIP_LOOPING);
clip->buffer_size = copy_size; atomic_store(&clip->buffer_size, copy_size);
clip->write_position = copy_size; clip->write_position = copy_size;
clip->read_position = 0; clip->read_position = 0;
@@ -700,19 +710,23 @@ void engine_push_undo_action(Engine *engine, UndoAction *action) {
UndoHistory *history = &engine->undo_history; UndoHistory *history = &engine->undo_history;
int undo_idx = atomic_load(&history->undo_index);
int redo_idx = atomic_load(&history->redo_index);
// If we've undone some actions, clear the redo history // If we've undone some actions, clear the redo history
if (history->redo_index > history->undo_index) { if (redo_idx > undo_idx) {
history->redo_index = history->undo_index; atomic_store(&history->redo_index, undo_idx);
} }
// Add action at current undo position // Add action at current undo position
int slot = history->undo_index % MAX_UNDO_HISTORY; int slot = undo_idx % MAX_UNDO_HISTORY;
history->actions[slot] = *action; history->actions[slot] = *action;
history->undo_index++; atomic_store(&history->undo_index, undo_idx + 1);
history->redo_index = history->undo_index; atomic_store(&history->redo_index, undo_idx + 1);
if (history->count < MAX_UNDO_HISTORY) { int count = atomic_load(&history->count);
history->count++; if (count < MAX_UNDO_HISTORY) {
atomic_store(&history->count, count + 1);
} }
} }
@@ -721,9 +735,10 @@ void engine_undo(Engine *engine) {
if (!engine) return; if (!engine) return;
UndoHistory *history = &engine->undo_history; UndoHistory *history = &engine->undo_history;
if (history->undo_index <= 0) return; // Nothing to undo int undo_idx = atomic_load(&history->undo_index);
if (undo_idx <= 0) return; // Nothing to undo
int slot = (history->undo_index - 1) % MAX_UNDO_HISTORY; int slot = (undo_idx - 1) % MAX_UNDO_HISTORY;
UndoAction *action = &history->actions[slot]; UndoAction *action = &history->actions[slot];
switch (action->type) { switch (action->type) {
@@ -809,7 +824,7 @@ void engine_undo(Engine *engine) {
} }
} }
history->undo_index--; atomic_store(&history->undo_index, atomic_load(&history->undo_index) - 1);
} }
// Redo the last undone action // Redo the last undone action
@@ -817,9 +832,11 @@ void engine_redo(Engine *engine) {
if (!engine) return; if (!engine) return;
UndoHistory *history = &engine->undo_history; UndoHistory *history = &engine->undo_history;
if (history->redo_index <= history->undo_index) return; // Nothing to redo int undo_idx = atomic_load(&history->undo_index);
int redo_idx = atomic_load(&history->redo_index);
if (redo_idx <= undo_idx) return; // Nothing to redo
int slot = history->undo_index % MAX_UNDO_HISTORY; int slot = undo_idx % MAX_UNDO_HISTORY;
UndoAction *action = &history->actions[slot]; UndoAction *action = &history->actions[slot];
switch (action->type) { switch (action->type) {
@@ -941,7 +958,7 @@ void engine_redo(Engine *engine) {
} }
} }
history->undo_index++; atomic_store(&history->undo_index, atomic_load(&history->undo_index) + 1);
} }
int engine_init(Engine *engine, const char *client_name) { int engine_init(Engine *engine, const char *client_name) {
@@ -955,9 +972,9 @@ int engine_init(Engine *engine, const char *client_name) {
engine->queued_triggers = NULL; engine->queued_triggers = NULL;
// Initialize undo history // Initialize undo history
engine->undo_history.undo_index = 0; atomic_store(&engine->undo_history.undo_index, 0);
engine->undo_history.redo_index = 0; atomic_store(&engine->undo_history.redo_index, 0);
engine->undo_history.count = 0; atomic_store(&engine->undo_history.count, 0);
// Initialize command queue // Initialize command queue
command_queue_init(&engine->command_queue); command_queue_init(&engine->command_queue);
@@ -983,19 +1000,19 @@ int engine_init(Engine *engine, const char *client_name) {
// Initialize clips // Initialize clips
for (int i = 0; i < MAX_CLIPS; i++) { for (int i = 0; i < MAX_CLIPS; i++) {
engine->clips[i].state = CLIP_EMPTY; atomic_store(&engine->clips[i].state, CLIP_EMPTY);
engine->clips[i].buffer = (float *)calloc(MAX_BUFFER_SIZE, sizeof(float)); engine->clips[i].buffer = (float *)calloc(MAX_BUFFER_SIZE, sizeof(float));
if (!engine->clips[i].buffer) { if (!engine->clips[i].buffer) {
// Cleanup on allocation failure // Cleanup on allocation failure
for (int j = 0; j < i; j++) { for (int j = 0; j < i; j++) {
free(engine->clips[j].buffer); free(engine->clips[j].buffer);
engine->clips[j].buffer = NULL; // ADD THIS engine->clips[j].buffer = NULL;
} }
free(engine->transport); free(engine->transport);
engine->transport = NULL; // ADD THIS engine->transport = NULL;
return -1; return -1;
} }
engine->clips[i].buffer_size = 0; atomic_store(&engine->clips[i].buffer_size, 0);
engine->clips[i].write_position = 0; engine->clips[i].write_position = 0;
engine->clips[i].read_position = 0; engine->clips[i].read_position = 0;
} }

View File

@@ -97,9 +97,9 @@ typedef struct {
typedef struct { typedef struct {
UndoAction actions[MAX_UNDO_HISTORY]; UndoAction actions[MAX_UNDO_HISTORY];
int undo_index; // Points to next action to undo atomic_int undo_index; // Points to next action to undo
int redo_index; // Points to next action to redo atomic_int redo_index; // Points to next action to redo
int count; // Total actions in history atomic_int count; // Total actions in history
} UndoHistory; } UndoHistory;
typedef struct { typedef struct {