From ca5bb796d35b7812f939ae71f29c64d46091d339 Mon Sep 17 00:00:00 2001 From: paul Date: Tue, 13 Apr 2010 20:33:28 +0000 Subject: [PATCH] fix a possible race/deadlock when jack is quitting and clients try to connect; mention jack_free() and not free() in port buffer docs; fix failure to execute clients using wait API; tweak session mgmt code so that the SM can pass directory names with or without a trailing '/'; unload/finish internal clients at a better time to stop crashes; add new and as-yet not used and definitely unfinished a2j internal clients for ALSA/MIDI bridge git-svn-id: svn+ssh://jackaudio.org/trunk/jack@3981 0c269be4-1314-0410-8aa9-9f06e86f4224 --- configure.ac | 3 +- drivers/Makefile.am | 5 +- drivers/alsa/alsa_driver.c | 27 ++++---- jack/engine.h | 1 + jack/internal.h | 15 +++-- jack/jack.h | 6 +- jack/midiport.h | 21 +++--- jackd/clientengine.c | 128 ++++++++++++++++++++----------------- jackd/engine.c | 59 +++++++++++------ 9 files changed, 150 insertions(+), 115 deletions(-) diff --git a/configure.ac b/configure.ac index 602a799..1a7979f 100644 --- a/configure.ac +++ b/configure.ac @@ -844,7 +844,7 @@ then [HAVE_ALSA="true" ALSA_LIBS=-lasound JACK_DEFAULT_DRIVER=\"alsa\" - ], AC_MSG_RESULT([no]), [-lm] + ], AC_MSG_RESULT([no - cannot find ALSA 1.0.18 or later]), [-lm] ) AC_SUBST(ALSA_LIBS) fi @@ -928,6 +928,7 @@ doc/reference.doxygen drivers/Makefile drivers/alsa/Makefile drivers/alsa-midi/Makefile +drivers/a2j/Makefile drivers/dummy/Makefile drivers/oss/Makefile drivers/sun/Makefile diff --git a/drivers/Makefile.am b/drivers/Makefile.am index 4d232c2..276684a 100644 --- a/drivers/Makefile.am +++ b/drivers/Makefile.am @@ -8,6 +8,7 @@ endif if HAVE_ALSA_MIDI ALSA_MIDI_DIR = alsa-midi +A2J_DIR = a2j else ALSA_MIDI_DIR = endif @@ -48,5 +49,5 @@ else FIREWIRE_DIR = endif -SUBDIRS = $(ALSA_MIDI_DIR) $(ALSA_DIR) dummy $(OSS_DIR) $(SUN_DIR) $(PA_DIR) $(CA_DIR) $(FREEBOB_DIR) $(FIREWIRE_DIR) netjack -DIST_SUBDIRS = alsa alsa-midi dummy oss sun portaudio coreaudio freebob firewire netjack +SUBDIRS = $(ALSA_MIDI_DIR) $(A2J_DIR) $(ALSA_DIR) dummy $(OSS_DIR) $(SUN_DIR) $(PA_DIR) $(CA_DIR) $(FREEBOB_DIR) $(FIREWIRE_DIR) netjack +DIST_SUBDIRS = alsa alsa-midi a2j dummy oss sun portaudio coreaudio freebob firewire netjack diff --git a/drivers/alsa/alsa_driver.c b/drivers/alsa/alsa_driver.c index fe7107b..13d476c 100644 --- a/drivers/alsa/alsa_driver.c +++ b/drivers/alsa/alsa_driver.c @@ -1456,21 +1456,18 @@ alsa_driver_null_cycle (alsa_driver_t* driver, jack_nframes_t nframes) if (nframes > driver->frames_per_cycle) { return -1; } - - if (driver->capture_handle) { + + if (driver->capture_handle) { nf = nframes; offset = 0; while (nf) { contiguous = nf; - if (snd_pcm_mmap_begin ( - driver->capture_handle, - &driver->capture_areas, - (snd_pcm_uframes_t *) &offset, - (snd_pcm_uframes_t *) &contiguous)) { - return -1; - } - + if (alsa_driver_get_channel_addresses (driver, + &contiguous, 0, &offset, 0)) { + return -1; + } + if (snd_pcm_mmap_commit (driver->capture_handle, offset, contiguous) < 0) { return -1; @@ -1486,11 +1483,11 @@ alsa_driver_null_cycle (alsa_driver_t* driver, jack_nframes_t nframes) while (nf) { contiguous = nf; - if (alsa_driver_get_channel_addresses (driver, - 0, &contiguous, 0, &offset)) { - return -1; - } - + if (alsa_driver_get_channel_addresses (driver, + 0, &contiguous, 0, &offset)) { + return -1; + } + for (chn = 0; chn < driver->playback_nchannels; chn++) { alsa_driver_silence_on_channel (driver, chn, contiguous); diff --git a/jack/engine.h b/jack/engine.h index 46e0d51..fd6eed8 100644 --- a/jack/engine.h +++ b/jack/engine.h @@ -136,6 +136,7 @@ struct _jack_engine { pid_t wait_pid; int nozombies; volatile int problems; + volatile int new_clients_allowed; /* these lists are protected by `client_lock' */ JSList *clients; diff --git a/jack/internal.h b/jack/internal.h index 801ddc9..62a7ccf 100644 --- a/jack/internal.h +++ b/jack/internal.h @@ -525,12 +525,17 @@ extern char *jack_default_server_name (void); void silent_jack_error_callback (const char *desc); /* needed for port management */ -jack_port_t *jack_port_by_id_int (const jack_client_t *client, - jack_port_id_t id, int* free); +extern jack_port_t *jack_port_by_id_int (const jack_client_t *client, + jack_port_id_t id, int* free); -jack_port_t *jack_port_by_name_int (jack_client_t *client, - const char *port_name); -int jack_port_name_equals (jack_port_shared_t* port, const char* target); +extern jack_port_t *jack_port_by_name_int (jack_client_t *client, + const char *port_name); +extern int jack_port_name_equals (jack_port_shared_t* port, const char* target); + +/** Get the size (in bytes) of the data structure used to store + * MIDI events internally. + */ +extern size_t jack_midi_internal_event_size (); #ifdef __GNUC__ # define likely(x) __builtin_expect((x),1) diff --git a/jack/jack.h b/jack/jack.h index 21174dc..3d51637 100644 --- a/jack/jack.h +++ b/jack/jack.h @@ -606,7 +606,7 @@ int jack_port_connected_to (const jack_port_t *port, * @return a null-terminated array of full port names to which the @a * port is connected. If none, returns NULL. * - * The caller is responsible for calling free(3) on any non-NULL + * The caller is responsible for calling jack_free(3) on any non-NULL * returned value. * * @param port locally owned jack_port_t pointer. @@ -619,7 +619,7 @@ const char **jack_port_get_connections (const jack_port_t *port) JACK_OPTIONAL_W * @return a null-terminated array of full port names to which the @a * port is connected. If none, returns NULL. * - * The caller is responsible for calling free(3) on any non-NULL + * The caller is responsible for calling jack_free(3) on any non-NULL * returned value. * * This differs from jack_port_get_connections() in two important @@ -869,7 +869,7 @@ int jack_port_type_size(void) JACK_OPTIONAL_WEAK_EXPORT; * If zero, no selection based on flags will be carried out. * * @return a NULL-terminated array of ports that match the specified - * arguments. The caller is responsible for calling free(3) any + * arguments. The caller is responsible for calling jack_free(3) any * non-NULL returned value. * * @see jack_port_name_size(), jack_port_type_size() diff --git a/jack/midiport.h b/jack/midiport.h index 042ad84..870648d 100644 --- a/jack/midiport.h +++ b/jack/midiport.h @@ -25,6 +25,7 @@ extern "C" { #endif +#include #include #include @@ -53,7 +54,7 @@ typedef struct _jack_midi_event * @return number of events inside @a port_buffer */ jack_nframes_t -jack_midi_get_event_count(void* port_buffer); +jack_midi_get_event_count(void* port_buffer) JACK_OPTIONAL_WEAK_EXPORT; /** Get a MIDI event from an event port buffer. @@ -70,7 +71,7 @@ jack_midi_get_event_count(void* port_buffer); int jack_midi_event_get(jack_midi_event_t *event, void *port_buffer, - jack_nframes_t event_index); + jack_nframes_t event_index) JACK_OPTIONAL_WEAK_EXPORT; /** Clear an event buffer. @@ -82,7 +83,7 @@ jack_midi_event_get(jack_midi_event_t *event, * @param port_buffer Port buffer to clear (must be an output port buffer). */ void -jack_midi_clear_buffer(void *port_buffer); +jack_midi_clear_buffer(void *port_buffer) JACK_OPTIONAL_WEAK_EXPORT; /** Get the size of the largest event that can be stored by the port. @@ -93,7 +94,7 @@ jack_midi_clear_buffer(void *port_buffer); * @param port_buffer Port buffer to check size of. */ size_t -jack_midi_max_event_size(void* port_buffer); +jack_midi_max_event_size(void* port_buffer) JACK_OPTIONAL_WEAK_EXPORT; /** Allocate space for an event to be written to an event port buffer. @@ -114,7 +115,7 @@ jack_midi_max_event_size(void* port_buffer); jack_midi_data_t* jack_midi_event_reserve(void *port_buffer, jack_nframes_t time, - size_t data_size); + size_t data_size) JACK_OPTIONAL_WEAK_EXPORT; /** Write an event into an event port buffer. @@ -133,7 +134,7 @@ int jack_midi_event_write(void *port_buffer, jack_nframes_t time, const jack_midi_data_t *data, - size_t data_size); + size_t data_size) JACK_OPTIONAL_WEAK_EXPORT; /** Get the number of events that could not be written to @a port_buffer. @@ -145,13 +146,7 @@ jack_midi_event_write(void *port_buffer, * @returns Number of events that could not be written to @a port_buffer. */ jack_nframes_t -jack_midi_get_lost_event_count(void *port_buffer); - -/** Get the size (in bytes) of the data structure used to store - * MIDI events internally. This is not useful for JACK applications. - */ -size_t -jack_midi_internal_event_size (); +jack_midi_get_lost_event_count(void *port_buffer) JACK_OPTIONAL_WEAK_EXPORT; /*@}*/ diff --git a/jackd/clientengine.c b/jackd/clientengine.c index 3be58fe..59ee175 100644 --- a/jackd/clientengine.c +++ b/jackd/clientengine.c @@ -89,6 +89,58 @@ jack_client_do_deactivate (jack_engine_t *engine, return 0; } +static int +jack_load_client (jack_engine_t *engine, jack_client_internal_t *client, + const char *so_name) +{ + const char *errstr; + char path_to_so[PATH_MAX+1]; + + snprintf (path_to_so, sizeof (path_to_so), ADDON_DIR "/%s.so", so_name); + client->handle = dlopen (path_to_so, RTLD_NOW|RTLD_GLOBAL); + + if (client->handle == 0) { + if ((errstr = dlerror ()) != 0) { + jack_error ("%s", errstr); + } else { + jack_error ("bizarre error loading %s", so_name); + } + return -1; + } + + client->initialize = dlsym (client->handle, "jack_initialize"); + + if ((errstr = dlerror ()) != 0) { + jack_error ("%s has no initialize() function\n", so_name); + dlclose (client->handle); + client->handle = 0; + return -1; + } + + client->finish = (void (*)(void *)) dlsym (client->handle, + "jack_finish"); + + if ((errstr = dlerror ()) != 0) { + jack_error ("%s has no finish() function", so_name); + dlclose (client->handle); + client->handle = 0; + return -1; + } + + return 0; +} + +static void +jack_client_unload (jack_client_internal_t *client) +{ + if (client->handle) { + if (client->finish) { + client->finish (client->private_client->process_arg); + } + dlclose (client->handle); + } +} + static void jack_zombify_client (jack_engine_t *engine, jack_client_internal_t *client) { @@ -115,6 +167,12 @@ jack_remove_client (jack_engine_t *engine, jack_client_internal_t *client) VERBOSE (engine, "removing client \"%s\"", client->control->name); + if (client->control->type == ClientInternal) { + /* unload it while its still a regular client */ + + jack_client_unload (client); + } + /* if its not already a zombie, make it so */ if (!client->control->dead) { @@ -141,7 +199,7 @@ jack_remove_client (jack_engine_t *engine, jack_client_internal_t *client) close (client->event_fd); close (client->request_fd); - } + } for (node = engine->clients; node; node = jack_slist_next (node)) { if (((jack_client_internal_t *) node->data)->control->id @@ -159,11 +217,17 @@ jack_remove_client (jack_engine_t *engine, jack_client_internal_t *client) if (engine->temporary && (jack_slist_length(engine->clients) <= 1)) { if (engine->wait_pid >= 0) { + /* block new clients from being created + after we release the lock. + */ + engine->new_clients_allowed = 0; /* tell the waiter we're done to initiate a normal shutdown. */ VERBOSE (engine, "Kill wait pid to stop"); kill (engine->wait_pid, SIGUSR2); + /* unlock the graph so that the server thread can finish */ + jack_unlock_graph (engine); sleep (-1); } else { exit (0); @@ -305,58 +369,6 @@ jack_remove_clients (jack_engine_t* engine, int* exit_freewheeling_when_done) VERBOSE (engine, "-- Removing failed clients ..."); } -static int -jack_load_client (jack_engine_t *engine, jack_client_internal_t *client, - const char *so_name) -{ - const char *errstr; - char path_to_so[PATH_MAX+1]; - - snprintf (path_to_so, sizeof (path_to_so), ADDON_DIR "/%s.so", so_name); - client->handle = dlopen (path_to_so, RTLD_NOW|RTLD_GLOBAL); - - if (client->handle == 0) { - if ((errstr = dlerror ()) != 0) { - jack_error ("%s", errstr); - } else { - jack_error ("bizarre error loading %s", so_name); - } - return -1; - } - - client->initialize = dlsym (client->handle, "jack_initialize"); - - if ((errstr = dlerror ()) != 0) { - jack_error ("%s has no initialize() function\n", so_name); - dlclose (client->handle); - client->handle = 0; - return -1; - } - - client->finish = (void (*)(void *)) dlsym (client->handle, - "jack_finish"); - - if ((errstr = dlerror ()) != 0) { - jack_error ("%s has no finish() function", so_name); - dlclose (client->handle); - client->handle = 0; - return -1; - } - - return 0; -} - -static void -jack_client_unload (jack_client_internal_t *client) -{ - if (client->handle) { - if (client->finish) { - client->finish (client->private_client->process_arg); - } - dlclose (client->handle); - } -} - jack_client_internal_t * jack_client_by_name (jack_engine_t *engine, const char *name) { @@ -1004,12 +1016,11 @@ jack_client_delete (jack_engine_t *engine, jack_client_internal_t *client) if (jack_client_is_internal (client)) { - jack_client_unload (client); free (client->private_client); free ((void *) client->control); - } else { - + } else { + /* release the client segment, mark it for destruction, and free up the shm registry information so that it can be reused. @@ -1017,9 +1028,10 @@ jack_client_delete (jack_engine_t *engine, jack_client_internal_t *client) jack_release_shm (&client->control_shm); jack_destroy_shm (&client->control_shm); - } + } + + free (client); - free (client); } void diff --git a/jackd/engine.c b/jackd/engine.c index 63bc1a4..3415c8a 100644 --- a/jackd/engine.c +++ b/jackd/engine.c @@ -861,7 +861,9 @@ jack_engine_process (jack_engine_t *engine, jack_nframes_t nframes) DEBUG ("considering client %s for processing", client->control->name); - if (!client->control->active || !client->control->process_cbset || client->control->dead) { + if (!client->control->active || + (!client->control->process_cbset && !client->control->thread_cb_cbset) || + client->control->dead) { node = jack_slist_next (node); } else if (jack_client_is_internal (client)) { node = jack_process_internal (engine, node, nframes); @@ -1671,7 +1673,7 @@ jack_server_thread (void *arg) &client_addrlen)) < 0) { jack_error ("cannot accept new connection (%s)", strerror (errno)); - } else if (jack_client_create (engine, client_socket) < 0) { + } else if (!engine->new_clients_allowed || jack_client_create (engine, client_socket) < 0) { jack_error ("cannot complete client " "connection process"); close (client_socket); @@ -1786,6 +1788,7 @@ jack_engine_new (int realtime, int rtpriority, int do_mlock, int do_unlock, engine->wait_pid = wait_pid; engine->nozombies = nozombies; engine->removing_clients = 0; + engine->new_clients_allowed = 1; engine->session_reply_fd = -1; engine->session_pending_replies = 0; @@ -2098,7 +2101,7 @@ jack_start_freewheeling (jack_engine_t* engine, jack_client_id_t client_id) client = jack_client_internal_by_id (engine, client_id); - if (client->control->process_cbset) { + if (client->control->process_cbset || client->control->thread_cb_cbset) { engine->fwclient = client_id; } @@ -2649,14 +2652,30 @@ jack_do_session_notify (jack_engine_t *engine, jack_request_t *req, int reply_fd for (node = engine->clients; node; node = jack_slist_next (node)) { jack_client_internal_t* client = (jack_client_internal_t*) node->data; if (client->control->session_cbset) { - + struct stat sbuf; + // in case we only want to send to a special client. // uuid assign is still complete. not sure if thats necessary. if( (req->x.session.target[0] != 0) && strcmp(req->x.session.target, (char *)client->control->name) ) continue; - snprintf (event.x.name, sizeof (event.x.name), "%s%s/", req->x.session.path, client->control->name ); - mkdir (event.x.name, 0777 ); + /* the caller of jack_session_notify() is required to have created the session dir + */ + + if (stat (req->x.session.path, &sbuf) != 0 || !S_ISDIR (sbuf.st_mode)) { + jack_error ("session parent directory (%s) does not exist", req->x.session.path); + goto error_out; + } + if (req->x.session.path[strlen(req->x.session.path)-1] == '/') { + snprintf (event.x.name, sizeof (event.x.name), "%s%s/", req->x.session.path, client->control->name ); + } else { + snprintf (event.x.name, sizeof (event.x.name), "%s/%s/", req->x.session.path, client->control->name ); + } + if (mkdir (event.x.name, 0777) != 0) { + jack_error ("cannot create session directory (%s) for client %s: %s", + event.x.name, client->control->name, strerror (errno)); + goto error_out; + } reply = jack_deliver_event (engine, client, &event); if (reply == 1) { @@ -2961,26 +2980,24 @@ jack_rechain_graph (jack_engine_t *engine) for (n = 0, node = engine->clients, next = NULL; node; node = next) { - next = jack_slist_next (node); + jack_client_internal_t* client = (jack_client_internal_t *) node->data; - if (! ((jack_client_internal_t *) node->data)->control->process_cbset) { + next = jack_slist_next (node); + + if (!client->control->process_cbset && !client->control->thread_cb_cbset) { continue; } VERBOSE(engine, "+++ client is now %s active ? %d", - ((jack_client_internal_t *) node->data)->control->name, - ((jack_client_internal_t *) node->data)->control->active); - - if (((jack_client_internal_t *) node->data)->control->active) { + client->control->name, client->control->active); - client = (jack_client_internal_t *) node->data; + if (client->control->active) { /* find the next active client. its ok for * this to be NULL */ while (next) { - if (((jack_client_internal_t *) - next->data)->control->active && ((jack_client_internal_t *)next->data)->control->process_cbset ) { + if (client->control->active && (client->control->process_cbset || client->control->thread_cb_cbset)) { break; } next = jack_slist_next (next); @@ -3465,12 +3482,13 @@ void jack_dump_configuration(jack_engine_t *engine, int take_lock) client = (jack_client_internal_t *) clientnode->data; ctl = client->control; - jack_info ("client #%d: %s (type: %d, process? %s," + jack_info ("client #%d: %s (type: %d, process? %s, thread ? %s" " start=%d wait=%d", ++n, ctl->name, ctl->type, ctl->process_cbset ? "yes" : "no", + ctl->thread_cb_cbset ? "yes" : "no", client->subgraph_start_fd, client->subgraph_wait_fd); @@ -4092,14 +4110,19 @@ jack_port_do_register (jack_engine_t *engine, jack_request_t *req, int internal) shared = &engine->control->ports[port_id]; - if (!internal || !engine->driver) + if (!internal || !engine->driver) { goto fallback; + } + + /* if the port belongs to the backend client, do some magic with names + */ backend_client_name = (char *) engine->driver->internal_client->control->name; len = strlen (backend_client_name); - if (strncmp (req->x.port_info.name, backend_client_name, len) != 0) + if (strncmp (req->x.port_info.name, backend_client_name, len) != 0) { goto fallback; + } /* use backend's original as an alias, use predefined names */