13
0

JACK backend: serialize all jack server calls with a mutex

As was noted in 88ee3af3ea it is unsafe/undefined behavior if two threads
sleep on the JACK request file descriptor, since there is no way to control
which one will wake and process the request. Since each thread may have
sent a different request, this can lead to a thread misinterpreting the
response because it is reading the wrong response.

This may (or may not) solve some subtle problems with JACK, but was
revealed by having a control surface (LaunchPad Pro) that registers
three ports from the butler thread at about the same as the GUI
thread is registering the auditioner. One thread read the wrong
response, and because of some slightly weird code/design, it attempts
to rename the port from within the response handler, which in JACK1
leads to deadlock (and later, zombification).
This commit is contained in:
Paul Davis 2023-09-13 09:20:41 -06:00
parent 88ee3af3ea
commit 3c857a78c6
3 changed files with 80 additions and 61 deletions

View File

@ -48,6 +48,8 @@ using std::vector;
#define GET_PRIVATE_JACK_POINTER(localvar) jack_client_t* localvar = _jack_connection->jack(); if (!(localvar)) { return; }
#define GET_PRIVATE_JACK_POINTER_RET(localvar,r) jack_client_t* localvar = _jack_connection->jack(); if (!(localvar)) { return r; }
// #define JACK_SERVER_CALL(expr) { std::cerr << "JACK SERVER CALL: " << pthread_self() << '/' << pthread_name() << ' ' << #expr << std::endl; Glib::Threads::Mutex::Lock lm (server_call_mutex); expr; }
#define JACK_SERVER_CALL(expr) { Glib::Threads::Mutex::Lock lm (server_call_mutex); expr; }
JACKAudioBackend::JACKAudioBackend (AudioEngine& e, AudioBackendInfo& info, std::shared_ptr<JackConnection> jc)
: AudioBackend (e, info)
@ -618,7 +620,11 @@ JACKAudioBackend::freewheel (bool onoff)
return 0;
}
if (jack_set_freewheel (_priv_jack, onoff) == 0) {
int ret;
JACK_SERVER_CALL (ret = jack_set_freewheel (_priv_jack, onoff));
if (ret == 0) {
_freewheeling = onoff;
return 0;
}
@ -669,9 +675,9 @@ JACKAudioBackend::set_time_master (bool yn)
{
GET_PRIVATE_JACK_POINTER_RET (_priv_jack, -1);
if (yn) {
return jack_set_timebase_callback (_priv_jack, 0, _jack_timebase_callback, this);
JACK_SERVER_CALL (return jack_set_timebase_callback (_priv_jack, 0, _jack_timebase_callback, this));
} else {
return jack_release_timebase (_priv_jack);
JACK_SERVER_CALL (return jack_release_timebase (_priv_jack));
}
}
@ -741,20 +747,21 @@ JACKAudioBackend::set_jack_callbacks ()
* non-callback API, and run the thread init callback in our own code.
*/
jack_set_process_thread (_priv_jack, _process_thread, this);
jack_set_sample_rate_callback (_priv_jack, _sample_rate_callback, this);
jack_set_buffer_size_callback (_priv_jack, _bufsize_callback, this);
jack_set_xrun_callback (_priv_jack, _xrun_callback, this);
jack_set_sync_callback (_priv_jack, _jack_sync_callback, this);
jack_set_freewheel_callback (_priv_jack, _freewheel_callback, this);
JACK_SERVER_CALL (jack_set_process_thread (_priv_jack, _process_thread, this));
JACK_SERVER_CALL (jack_set_sample_rate_callback (_priv_jack, _sample_rate_callback, this));
JACK_SERVER_CALL (jack_set_buffer_size_callback (_priv_jack, _bufsize_callback, this));
JACK_SERVER_CALL (jack_set_xrun_callback (_priv_jack, _xrun_callback, this));
JACK_SERVER_CALL (jack_set_sync_callback (_priv_jack, _jack_sync_callback, this));
JACK_SERVER_CALL (jack_set_freewheel_callback (_priv_jack, _freewheel_callback, this));
#ifdef HAVE_JACK_SESSION
if( jack_set_session_callback)
jack_set_session_callback (_priv_jack, _session_callback, this);
if (jack_set_session_callback) {
JACK_SERVER_CALL (jack_set_session_callback (_priv_jack, _session_callback, this));
}
#endif
if (jack_set_latency_callback) {
jack_set_latency_callback (_priv_jack, _latency_callback, this);
JACK_SERVER_CALL (jack_set_latency_callback (_priv_jack, _latency_callback, this));
}
jack_set_error_function (ardour_jack_error);
@ -764,6 +771,7 @@ void
JACKAudioBackend::_jack_timebase_callback (jack_transport_state_t state, pframes_t nframes,
jack_position_t* pos, int new_position, void *arg)
{
DEBUG_TRACE (DEBUG::BackendCallbacks, string_compose ("%1/%2 jack timebase callback\n", pthread_self(), pthread_name()));
static_cast<JACKAudioBackend*> (arg)->jack_timebase_callback (state, nframes, pos, new_position);
}
@ -782,6 +790,7 @@ JACKAudioBackend::jack_timebase_callback (jack_transport_state_t state, pframes_
int
JACKAudioBackend::_jack_sync_callback (jack_transport_state_t state, jack_position_t* pos, void* arg)
{
DEBUG_TRACE (DEBUG::BackendCallbacks, string_compose ("%1/%2 jack sync callback\n", pthread_self(), pthread_name()));
return static_cast<JACKAudioBackend*> (arg)->jack_sync_callback (state, pos);
}
@ -820,6 +829,8 @@ JACKAudioBackend::jack_sync_callback (jack_transport_state_t state, jack_positio
int
JACKAudioBackend::_xrun_callback (void *arg)
{
DEBUG_TRACE (DEBUG::BackendCallbacks, string_compose ("%1/%2 jack sync callback\n", pthread_self(), pthread_name()));
JACKAudioBackend* jab = static_cast<JACKAudioBackend*> (arg);
if (jab->available()) {
jab->engine.Xrun (); /* EMIT SIGNAL */
@ -830,6 +841,8 @@ JACKAudioBackend::_xrun_callback (void *arg)
void
JACKAudioBackend::_session_callback (jack_session_event_t *event, void *arg)
{
DEBUG_TRACE (DEBUG::BackendCallbacks, string_compose ("%1/%2 jack session callback\n", pthread_self(), pthread_name()));
JACKAudioBackend* jab = static_cast<JACKAudioBackend*> (arg);
ARDOUR::Session* session = jab->engine.session();
@ -842,6 +855,7 @@ JACKAudioBackend::_session_callback (jack_session_event_t *event, void *arg)
void
JACKAudioBackend::_freewheel_callback (int onoff, void *arg)
{
DEBUG_TRACE (DEBUG::BackendCallbacks, string_compose ("%1/%2 jack freewheel callback\n", pthread_self(), pthread_name()));
static_cast<JACKAudioBackend*>(arg)->freewheel_callback (onoff);
}
@ -855,6 +869,7 @@ JACKAudioBackend::freewheel_callback (int onoff)
void
JACKAudioBackend::_latency_callback (jack_latency_callback_mode_t mode, void* arg)
{
DEBUG_TRACE (DEBUG::BackendCallbacks, string_compose ("%1/%2 jack latency callback\n", pthread_self(), pthread_name()));
return static_cast<JACKAudioBackend*> (arg)->jack_latency_callback (mode);
}
@ -882,16 +897,15 @@ JACKAudioBackend::join_process_threads ()
int ret = 0;
for (std::vector<jack_native_thread_t>::const_iterator i = _jack_threads.begin ();
i != _jack_threads.end(); i++) {
for (auto & thread : _jack_threads) {
#if defined(USING_JACK2_EXPANSION_OF_JACK_API) || defined __jack_systemdeps_h__
// jack_client is not used by JACK2's implementation
// also jack_client_close() leaves threads active
if (jack_client_stop_thread (_priv_jack, *i) != 0)
if (jack_client_stop_thread (_priv_jack, thread) != 0)
#else
void* status;
if (pthread_join (*i, &status) != 0)
if (pthread_join (thread, &status) != 0)
#endif
{
error << "AudioEngine: cannot stop process thread" << endmsg;
@ -917,15 +931,14 @@ JACKAudioBackend::in_process_thread ()
}
#endif
for (std::vector<jack_native_thread_t>::const_iterator i = _jack_threads.begin ();
i != _jack_threads.end(); i++) {
for (auto & thread : _jack_threads) {
#if defined COMPILER_MINGW && (!defined PTW32_VERSION || defined __jack_systemdeps_h__)
if (*i == GetCurrentThread()) {
if (thread == GetCurrentThread()) {
return true;
}
#else // pthreads
if (pthread_equal (*i, pthread_self()) != 0) {
if (pthread_equal (thread, pthread_self()) != 0) {
return true;
}
#endif
@ -1001,6 +1014,7 @@ JACKAudioBackend::process_thread ()
int
JACKAudioBackend::_sample_rate_callback (pframes_t nframes, void *arg)
{
DEBUG_TRACE (DEBUG::BackendCallbacks, string_compose ("%1/%2 jack sample rate callback\n", pthread_self(), pthread_name()));
return static_cast<JACKAudioBackend*> (arg)->jack_sample_rate_callback (nframes);
}
@ -1020,6 +1034,7 @@ JACKAudioBackend::jack_latency_callback (jack_latency_callback_mode_t mode)
int
JACKAudioBackend::_bufsize_callback (pframes_t nframes, void *arg)
{
DEBUG_TRACE (DEBUG::BackendCallbacks, string_compose ("%1/%2 jack buffer size callback\n", pthread_self(), pthread_name()));
return static_cast<JACKAudioBackend*> (arg)->jack_bufsize_callback (nframes);
}
@ -1086,7 +1101,7 @@ void
JACKAudioBackend::update_latencies ()
{
GET_PRIVATE_JACK_POINTER (_priv_jack);
jack_recompute_total_latencies (_priv_jack);
JACK_SERVER_CALL (jack_recompute_total_latencies (_priv_jack));
}
ChanCount

View File

@ -324,7 +324,7 @@ class JACKAudioBackend : public AudioBackend {
JACKSession* _session;
Glib::Threads::Mutex port_registration_mutex;
mutable Glib::Threads::Mutex server_call_mutex;
protected:
int _start (bool for_latency_measurement);

View File

@ -26,6 +26,7 @@
#include "jack_audiobackend.h"
#include "jack_connection.h"
#include "ardour/debug.h"
#include "ardour/port_manager.h"
#include "pbd/i18n.h"
@ -37,6 +38,8 @@ using std::vector;
#define GET_PRIVATE_JACK_POINTER(localvar) jack_client_t* localvar = _jack_connection->jack(); if (!(localvar)) { return; }
#define GET_PRIVATE_JACK_POINTER_RET(localvar,r) jack_client_t* localvar = _jack_connection->jack(); if (!(localvar)) { return r; }
// #define JACK_SERVER_CALL(expr) { std::cerr << "JACK SERVER CALL: " << pthread_self() << '/' << pthread_name() << ' ' << #expr << std::endl; Glib::Threads::Mutex::Lock lm (server_call_mutex); expr; }
#define JACK_SERVER_CALL(expr) { Glib::Threads::Mutex::Lock lm (server_call_mutex); expr; }
static uint32_t
ardour_port_flags_to_jack_flags (PortFlags flags)
@ -99,9 +102,9 @@ JACKAudioBackend::when_connected_to_jack ()
return;
}
jack_set_port_registration_callback (client, _registration_callback, this);
jack_set_port_connect_callback (client, _connect_callback, this);
jack_set_graph_order_callback (client, _graph_order_callback, this);
JACK_SERVER_CALL (jack_set_port_registration_callback (client, _registration_callback, this));
JACK_SERVER_CALL (jack_set_port_connect_callback (client, _connect_callback, this));
JACK_SERVER_CALL (jack_set_graph_order_callback (client, _graph_order_callback, this));
}
int
@ -110,12 +113,12 @@ JACKAudioBackend::set_port_name (PortHandle port, const std::string& name)
#if HAVE_JACK_PORT_RENAME
jack_client_t* client = _jack_connection->jack();
if (client) {
return jack_port_rename (client, std::dynamic_pointer_cast<JackPort>(port)->jack_ptr, name.c_str());
JACK_SERVER_CALL (return jack_port_rename (client, std::dynamic_pointer_cast<JackPort>(port)->jack_ptr, name.c_str()));
} else {
return -1;
}
#else
return jack_port_set_name (std::dynamic_pointer_cast<JackPort>(port)->jack_ptr, name.c_str());
JACK_SERVER_CALL (return jack_port_set_name (std::dynamic_pointer_cast<JackPort>(port)->jack_ptr, name.c_str()));
#endif
}
@ -224,6 +227,8 @@ JACKAudioBackend::get_port_by_name (const std::string& name) const
void
JACKAudioBackend::_registration_callback (jack_port_id_t id, int reg, void* arg)
{
DEBUG_TRACE (DEBUG::BackendCallbacks, string_compose ("%1/%2 jack port registration callback\n", pthread_self(), pthread_name()));
/* we don't use a virtual method for the registration callback, because
JACK is the only backend that delivers the arguments shown above. So
call our own JACK-centric registration callback, then the generic
@ -292,12 +297,14 @@ JACKAudioBackend::jack_registration_callback (jack_port_id_t id, int reg)
int
JACKAudioBackend::_graph_order_callback (void *arg)
{
DEBUG_TRACE (DEBUG::BackendCallbacks, string_compose ("%1/%2 jack graph order callback\n", pthread_self(), pthread_name()));
return static_cast<JACKAudioBackend*> (arg)->manager.graph_order_callback ();
}
void
JACKAudioBackend::_connect_callback (jack_port_id_t id_a, jack_port_id_t id_b, int conn, void* arg)
{
DEBUG_TRACE (DEBUG::BackendCallbacks, string_compose ("%1/%2 jack connect/disconnect callback\n", pthread_self(), pthread_name()));
static_cast<JACKAudioBackend*> (arg)->connect_callback (id_a, id_b, conn);
}
@ -327,7 +334,7 @@ JACKAudioBackend::connected (PortHandle p, bool process_callback_safe)
ports = jack_port_get_connections (port);
} else {
GET_PRIVATE_JACK_POINTER_RET (_priv_jack, false);
ports = jack_port_get_all_connections (_priv_jack, port);
JACK_SERVER_CALL (ports = jack_port_get_all_connections (_priv_jack, port));
}
if (ports) {
@ -350,7 +357,7 @@ JACKAudioBackend::connected_to (PortHandle p, const std::string& other, bool pro
ports = jack_port_get_connections (port);
} else {
GET_PRIVATE_JACK_POINTER_RET (_priv_jack, false);
ports = jack_port_get_all_connections (_priv_jack, port);
JACK_SERVER_CALL (ports = jack_port_get_all_connections (_priv_jack, port));
}
if (ports) {
@ -377,7 +384,7 @@ JACKAudioBackend::physically_connected (PortHandle p, bool process_callback_safe
ports = jack_port_get_connections ((jack_port_t*)port);
} else {
GET_PRIVATE_JACK_POINTER_RET (_priv_jack, false);
ports = jack_port_get_all_connections (_priv_jack, (jack_port_t*)port);
JACK_SERVER_CALL (ports = jack_port_get_all_connections (_priv_jack, (jack_port_t*)port));
}
if (ports) {
@ -408,7 +415,7 @@ JACKAudioBackend::externally_connected (PortHandle p, bool process_callback_safe
ports = jack_port_get_connections ((jack_port_t*)port);
} else {
GET_PRIVATE_JACK_POINTER_RET (_priv_jack, false);
ports = jack_port_get_all_connections (_priv_jack, (jack_port_t*)port);
JACK_SERVER_CALL (ports = jack_port_get_all_connections (_priv_jack, (jack_port_t*)port));
}
if (ports) {
@ -439,7 +446,7 @@ JACKAudioBackend::get_connections (PortHandle p, vector<string>& s, bool process
ports = jack_port_get_connections (port);
} else {
GET_PRIVATE_JACK_POINTER_RET (_priv_jack, 0);
ports = jack_port_get_all_connections (_priv_jack, port);
JACK_SERVER_CALL (ports = jack_port_get_all_connections (_priv_jack, port));
}
if (ports) {
@ -480,9 +487,7 @@ JACKAudioBackend::get_ports (const string& port_name_pattern, DataType type, Por
GET_PRIVATE_JACK_POINTER_RET (_priv_jack,0);
const char** ports = jack_get_ports (_priv_jack, port_name_pattern.c_str(),
ardour_data_type_to_jack_port_type (type),
ardour_port_flags_to_jack_flags (flags));
const char** ports = jack_get_ports (_priv_jack, port_name_pattern.c_str(), ardour_data_type_to_jack_port_type (type), ardour_port_flags_to_jack_flags (flags));
if (ports == 0) {
return 0;
@ -513,21 +518,19 @@ void
JACKAudioBackend::get_physical (DataType type, unsigned long flags, vector<string>& phy) const
{
GET_PRIVATE_JACK_POINTER (_priv_jack);
const char ** ports;
const char ** ports = jack_get_ports (_priv_jack, NULL, ardour_data_type_to_jack_port_type (type), JackPortIsPhysical | flags);
if ((ports = jack_get_ports (_priv_jack, NULL, ardour_data_type_to_jack_port_type (type), JackPortIsPhysical | flags)) == 0) {
if (!ports) {
return;
}
if (ports) {
for (uint32_t i = 0; ports[i]; ++i) {
if (strstr (ports[i], "Midi-Through")) {
continue;
}
phy.push_back (ports[i]);
for (uint32_t i = 0; ports[i]; ++i) {
if (strstr (ports[i], "Midi-Through")) {
continue;
}
jack_free (ports);
phy.push_back (ports[i]);
}
jack_free (ports);
}
/** Get physical ports for which JackPortIsOutput is set; ie those that correspond to
@ -553,9 +556,9 @@ bool
JACKAudioBackend::can_monitor_input () const
{
GET_PRIVATE_JACK_POINTER_RET (_priv_jack,false);
const char ** ports;
const char ** ports = jack_get_ports (_priv_jack, NULL, JACK_DEFAULT_AUDIO_TYPE, JackPortCanMonitor);
if ((ports = jack_get_ports (_priv_jack, NULL, JACK_DEFAULT_AUDIO_TYPE, JackPortCanMonitor)) == 0) {
if (ports) {
return false;
}
@ -567,12 +570,12 @@ JACKAudioBackend::can_monitor_input () const
int
JACKAudioBackend::request_input_monitoring (PortHandle port, bool yn)
{
return jack_port_request_monitor (std::dynamic_pointer_cast<JackPort>(port)->jack_ptr, yn);
JACK_SERVER_CALL (return jack_port_request_monitor (std::dynamic_pointer_cast<JackPort>(port)->jack_ptr, yn));
}
int
JACKAudioBackend::ensure_input_monitoring (PortHandle port, bool yn)
{
return jack_port_ensure_monitor (std::dynamic_pointer_cast<JackPort>(port)->jack_ptr, yn);
JACK_SERVER_CALL (return jack_port_ensure_monitor (std::dynamic_pointer_cast<JackPort>(port)->jack_ptr, yn));
}
bool
JACKAudioBackend::monitoring_input (PortHandle port)
@ -584,14 +587,9 @@ PortEngine::PortPtr
JACKAudioBackend::register_port (const std::string& shortname, ARDOUR::DataType type, ARDOUR::PortFlags flags)
{
jack_port_t* jack_port;
{
Glib::Threads::Mutex::Lock lm (port_registration_mutex);
GET_PRIVATE_JACK_POINTER_RET (_priv_jack, PortEngine::PortPtr());
jack_port = jack_port_register (_priv_jack, shortname.c_str(),
ardour_data_type_to_jack_port_type (type),
ardour_port_flags_to_jack_flags (flags),
0);
}
GET_PRIVATE_JACK_POINTER_RET (_priv_jack, PortEngine::PortPtr());
JACK_SERVER_CALL (jack_port = jack_port_register (_priv_jack, shortname.c_str(), ardour_data_type_to_jack_port_type (type), ardour_port_flags_to_jack_flags (flags), 0));
if (!jack_port) {
return PortEngine::PortPtr();
@ -635,7 +633,10 @@ int
JACKAudioBackend::connect (PortHandle port, const std::string& other)
{
GET_PRIVATE_JACK_POINTER_RET (_priv_jack, -1);
int r = jack_connect (_priv_jack, jack_port_name (std::dynamic_pointer_cast<JackPort>(port)->jack_ptr), other.c_str());
int r;
JACK_SERVER_CALL (r = jack_connect (_priv_jack, jack_port_name (std::dynamic_pointer_cast<JackPort>(port)->jack_ptr), other.c_str()));
if (r == 0 || r == EEXIST) {
return 0;
}
@ -645,11 +646,14 @@ int
JACKAudioBackend::connect (const std::string& src, const std::string& dst)
{
GET_PRIVATE_JACK_POINTER_RET (_priv_jack, -1);
int r;
JACK_SERVER_CALL (r = jack_connect (_priv_jack, src.c_str(), dst.c_str()));
int r = jack_connect (_priv_jack, src.c_str(), dst.c_str());
if (r == 0 || r == EEXIST) {
return 0;
}
return r;
}
@ -657,21 +661,21 @@ int
JACKAudioBackend::disconnect (PortHandle port, const std::string& other)
{
GET_PRIVATE_JACK_POINTER_RET (_priv_jack, -1);
return jack_disconnect (_priv_jack, jack_port_name (std::dynamic_pointer_cast<JackPort>(port)->jack_ptr), other.c_str());
JACK_SERVER_CALL (return jack_disconnect (_priv_jack, jack_port_name (std::dynamic_pointer_cast<JackPort>(port)->jack_ptr), other.c_str()));
}
int
JACKAudioBackend::disconnect (const std::string& src, const std::string& dst)
{
GET_PRIVATE_JACK_POINTER_RET (_priv_jack, -1);
return jack_disconnect (_priv_jack, src.c_str(), dst.c_str());
JACK_SERVER_CALL (return jack_disconnect (_priv_jack, src.c_str(), dst.c_str()));
}
int
JACKAudioBackend::disconnect_all (PortHandle port)
{
GET_PRIVATE_JACK_POINTER_RET (_priv_jack, -1);
return jack_port_disconnect (_priv_jack, std::dynamic_pointer_cast<JackPort>(port)->jack_ptr);
JACK_SERVER_CALL (return jack_port_disconnect (_priv_jack, std::dynamic_pointer_cast<JackPort>(port)->jack_ptr));
}
int