Skip to content

Commit

Permalink
linux-jack: fix deadlock when closing the client
Browse files Browse the repository at this point in the history
This lock causes a deadlock when freeing the JACK client while a
process callback is pending:

deactivate_jack -> locks mutex
   JACK thread -> calls jack_process_callback
   jack_process_callback -> blocks on mutex
jack_client_close -> joins JACK thread
(deadlock as the process callback never returns)

Instead, just don't lock the mutex. This is only mutexing on
creation/destruction of the JACK client. This is not necessary: the
process callback will only run after jack_activate (which is right
before the mutex is released in jack_init()), and will stop running
by the time jack_client_close returns. We don't actually need to
unregister any ports, so just call jack_client_close first thing in
deactivate_jack, which will guarantee the process callback has
completed before returning.

In fact, jack_process_callback isn't allowed to lock any mutexes or
allocate any memory at all, so this plug-in is still broken in this way
as obs_source_output_audio does that. This can cause audio xruns, as
realtime guarantees are violated. This is something that should be
fixed in the future, but at least it's not a deadlock.
  • Loading branch information
marcan authored and jp9000 committed Jan 3, 2021
1 parent 5d6bca0 commit 9616b79
Showing 1 changed file with 3 additions and 10 deletions.
13 changes: 3 additions & 10 deletions plugins/linux-jack/jack-wrapper.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ int jack_process_callback(jack_nframes_t nframes, void *arg)
if (data == 0)
return 0;

pthread_mutex_lock(&data->jack_mutex);

struct obs_source_audio out;
out.speakers = jack_channels_to_obs_speakers(data->channels);
out.samples_per_sec = jack_get_sample_rate(data->jack_client);
Expand All @@ -96,8 +94,9 @@ int jack_process_callback(jack_nframes_t nframes, void *arg)
"jack_get_cycle_times error: guessing timestamp");
}

/* FIXME: this function is not realtime-safe, we should do something
* about this */
obs_source_output_audio(data->source, &out);
pthread_mutex_unlock(&data->jack_mutex);
return 0;
}

Expand Down Expand Up @@ -164,17 +163,11 @@ void deactivate_jack(struct jack_data *data)
pthread_mutex_lock(&data->jack_mutex);

if (data->jack_client) {
jack_client_close(data->jack_client);
if (data->jack_ports != NULL) {
for (int i = 0; i < data->channels; ++i) {
if (data->jack_ports[i] != NULL)
jack_port_unregister(
data->jack_client,
data->jack_ports[i]);
}
bfree(data->jack_ports);
data->jack_ports = NULL;
}
jack_client_close(data->jack_client);
data->jack_client = NULL;
}
pthread_mutex_unlock(&data->jack_mutex);
Expand Down

0 comments on commit 9616b79

Please sign in to comment.