From cbfe31c02f108af5b59b7611cd02d78bde1e3938 Mon Sep 17 00:00:00 2001 From: paul Date: Tue, 26 Jan 2010 19:28:44 +0000 Subject: [PATCH] commit serious design flaw that left jackd in freewheeling mode if the client that initiated freewheeling leaves the graph (either deliberately, or via a crash) git-svn-id: svn+ssh://jackaudio.org/trunk/jack@3877 0c269be4-1314-0410-8aa9-9f06e86f4224 --- configure.ac | 2 +- jack/engine.h | 2 + jackd/clientengine.c | 9 +- jackd/clientengine.h | 2 +- jackd/engine.c | 216 ++++++++++++++++++++++++++++--------------- libjack/client.c | 1 + 6 files changed, 153 insertions(+), 79 deletions(-) diff --git a/configure.ac b/configure.ac index b434086..602a799 100644 --- a/configure.ac +++ b/configure.ac @@ -62,7 +62,7 @@ dnl changes are made dnl --- JACK_MAJOR_VERSION=0 JACK_MINOR_VERSION=118 -JACK_MICRO_VERSION=2 +JACK_MICRO_VERSION=3 dnl --- dnl HOWTO: updating the jack protocol version diff --git a/jack/engine.h b/jack/engine.h index 23f7ee1..438b13b 100644 --- a/jack/engine.h +++ b/jack/engine.h @@ -112,6 +112,7 @@ struct _jack_engine { unsigned long external_client_cnt; int rtpriority; char freewheeling; + jack_client_id_t fwclient; char verbose; char do_munlock; const char *server_name; @@ -231,5 +232,6 @@ void jack_port_clear_connections (jack_engine_t *engine, void jack_port_registration_notify (jack_engine_t *, jack_port_id_t, int); void jack_port_release (jack_engine_t *engine, jack_port_internal_t *); void jack_sort_graph (jack_engine_t *engine); +int jack_stop_freewheeling (jack_engine_t* engine, int engine_exiting); #endif /* __jack_engine_h__ */ diff --git a/jackd/clientengine.c b/jackd/clientengine.c index 4e245ca..a1599e1 100644 --- a/jackd/clientengine.c +++ b/jackd/clientengine.c @@ -216,7 +216,7 @@ jack_check_clients (jack_engine_t* engine, int with_timeout_check) } void -jack_remove_clients (jack_engine_t* engine) +jack_remove_clients (jack_engine_t* engine, int* exit_freewheeling_when_done) { JSList *tmp, *node; int need_sort = FALSE; @@ -238,6 +238,11 @@ jack_remove_clients (jack_engine_t* engine) if (client->error) { + if (engine->freewheeling && client->control->id == engine->fwclient) { + VERBOSE (engine, "freewheeling client has errors"); + *exit_freewheeling_when_done = 1; + } + /* if we have a communication problem with the client, remove it. otherwise, turn it into a zombie. the client will/should realize @@ -269,7 +274,7 @@ jack_remove_clients (jack_engine_t* engine) client->error = 0; } } - + need_sort = TRUE; } diff --git a/jackd/clientengine.h b/jackd/clientengine.h index c04d7a5..b6cf3a8 100644 --- a/jackd/clientengine.h +++ b/jackd/clientengine.h @@ -59,6 +59,6 @@ void jack_intclient_name_request (jack_engine_t *engine, void jack_intclient_unload_request (jack_engine_t *engine, jack_request_t *req); void jack_check_clients (jack_engine_t* engine, int with_timeout_check); -void jack_remove_clients (jack_engine_t* engine); +void jack_remove_clients (jack_engine_t* engine, int* exit_freewheeling); void jack_client_registration_notify (jack_engine_t *engine, const char* name, int yn); diff --git a/jackd/engine.c b/jackd/engine.c index 70f61a6..f1b659e 100644 --- a/jackd/engine.c +++ b/jackd/engine.c @@ -120,11 +120,12 @@ static void jack_engine_post_process (jack_engine_t *); static int jack_use_driver (jack_engine_t *engine, jack_driver_t *driver); static int jack_run_cycle (jack_engine_t *engine, jack_nframes_t nframes, float delayed_usecs); +static int jack_run_one_cycle (jack_engine_t *engine, jack_nframes_t nframes, + float delayed_usecs); static void jack_engine_delay (jack_engine_t *engine, float delayed_usecs); static void jack_engine_driver_exit (jack_engine_t* engine); -static int jack_start_freewheeling (jack_engine_t* engine); -static int jack_stop_freewheeling (jack_engine_t* engine); +static int jack_start_freewheeling (jack_engine_t* engine, jack_client_id_t); static int jack_client_feeds_transitive (jack_client_internal_t *source, jack_client_internal_t *dest); static int jack_client_sort (jack_client_internal_t *a, @@ -133,6 +134,7 @@ static void jack_check_acyclic (jack_engine_t* engine); static void jack_compute_all_port_total_latencies (jack_engine_t *engine); static void jack_compute_port_total_latency (jack_engine_t *engine, jack_port_shared_t*); static void jack_engine_signal_problems (jack_engine_t* engine); +static int jack_check_client_status (jack_engine_t* engine); static inline int jack_rolling_interval (jack_time_t period_usecs) @@ -577,7 +579,7 @@ jack_process_internal(jack_engine_t *engine, JSList *node, /* internal client */ - DEBUG ("invoking an internal client's callbacks"); + DEBUG ("invoking an internal client's (%s) callbacks", ctl->name); ctl->state = Running; engine->current_client = client; @@ -703,7 +705,7 @@ jack_process_external(jack_engine_t *engine, JSList *node) then = jack_get_microseconds (); if (engine->freewheeling) { - poll_timeout_usecs = 10000000; /* 10 seconds */ + poll_timeout_usecs = 250000; /* 0.25 seconds */ } else { poll_timeout_usecs = (engine->client_timeout_msecs > 0 ? engine->client_timeout_msecs * 1000 : @@ -742,6 +744,17 @@ jack_process_external(jack_engine_t *engine, JSList *node) decided that time was up ... */ + if (engine->freewheeling) { + if (jack_check_client_status (engine)) { + return NULL; + } else { + /* all clients are fine - we're just not done yet. since + we're freewheeling, that is fine. + */ + goto again; + } + } + #ifdef __linux if (linux_poll_bug_encountered (engine, then, &poll_timeout_usecs)) { goto again; @@ -1297,11 +1310,11 @@ do_request (jack_engine_t *engine, jack_request_t *req, int *reply_fd) break; case FreeWheel: - req->status = jack_start_freewheeling (engine); + req->status = jack_start_freewheeling (engine, req->x.client_id); break; case StopFreeWheel: - req->status = jack_stop_freewheeling (engine); + req->status = jack_stop_freewheeling (engine, 0); break; case SetBufferSize: @@ -1472,6 +1485,7 @@ jack_server_thread (void *arg) int done = 0; int i; const int fixed_fd_cnt = 3; + int stop_freewheeling; while (!done) { JSList* node; @@ -1582,12 +1596,17 @@ jack_server_thread (void *arg) and reset engine->problems */ + stop_freewheeling = 0; + while (problemsProblemsPROBLEMS) { VERBOSE (engine, "trying to lock graph to remove %d problems", problemsProblemsPROBLEMS); jack_lock_graph (engine); VERBOSE (engine, "we have problem clients (problems = %d", problemsProblemsPROBLEMS); - jack_remove_clients (engine); + jack_remove_clients (engine, &stop_freewheeling); + if (stop_freewheeling) { + VERBOSE (engine, "need to stop freewheeling once problems are cleared"); + } jack_unlock_graph (engine); jack_lock_problems (engine); @@ -1598,6 +1617,9 @@ jack_server_thread (void *arg) VERBOSE (engine, "after removing clients, problems = %d", problemsProblemsPROBLEMS); } + if (engine->freewheeling && stop_freewheeling) { + jack_stop_freewheeling (engine, 0); + } /* check the master server socket */ @@ -1727,6 +1749,7 @@ jack_engine_new (int realtime, int rtpriority, int do_mlock, int do_unlock, engine->server_name = server_name; engine->temporary = temporary; engine->freewheeling = 0; + engine->fwclient = 0; engine->feedbackcount = 0; engine->wait_pid = wait_pid; engine->nozombies = nozombies; @@ -1986,6 +2009,7 @@ static void* jack_engine_freewheel (void *arg) { jack_engine_t* engine = (jack_engine_t *) arg; + jack_client_internal_t* client; VERBOSE (engine, "freewheel thread starting ..."); @@ -1993,20 +2017,19 @@ jack_engine_freewheel (void *arg) have to do anything about scheduling. */ + client = jack_client_internal_by_id (engine, engine->fwclient); + while (engine->freewheeling) { - jack_lock_graph (engine); + jack_run_one_cycle (engine, engine->control->buffer_size, 0.0f); - if (jack_engine_process (engine, - engine->control->buffer_size)) { - jack_error ("process cycle within freewheel failed"); - jack_unlock_graph (engine); + if (client && client->error) { + /* run one cycle() will already have told the server thread + about issues, and the server thread will clean up. + however, its time for us to depart this world ... + */ break; } - - jack_engine_post_process (engine); - - jack_unlock_graph (engine); } VERBOSE (engine, "freewheel came to an end, naturally"); @@ -2014,9 +2037,10 @@ jack_engine_freewheel (void *arg) } static int -jack_start_freewheeling (jack_engine_t* engine) +jack_start_freewheeling (jack_engine_t* engine, jack_client_id_t client_id) { jack_event_t event; + jack_client_internal_t *client; if (engine->freewheeling) { return 0; @@ -2036,6 +2060,12 @@ jack_start_freewheeling (jack_engine_t* engine) return -1; } + client = jack_client_internal_by_id (engine, client_id); + + if (client->control->process_cbset) { + engine->fwclient = client_id; + } + engine->freewheeling = 1; event.type = StartFreewheel; @@ -2050,8 +2080,8 @@ jack_start_freewheeling (jack_engine_t* engine) return 0; } -static int -jack_stop_freewheeling (jack_engine_t* engine) +int +jack_stop_freewheeling (jack_engine_t* engine, int engine_exiting) { jack_event_t event; void *ftstatus; @@ -2074,23 +2104,68 @@ jack_stop_freewheeling (jack_engine_t* engine) to exit. */ + engine->fwclient = 0; engine->freewheeling = 0; + VERBOSE (engine, "freewheeling stopped, waiting for thread"); pthread_join (engine->freewheel_thread, &ftstatus); VERBOSE (engine, "freewheel thread has returned"); - /* tell everyone we've stopped */ + if (!engine_exiting) { + /* tell everyone we've stopped */ + + event.type = StopFreewheel; + jack_deliver_event_to_all (engine, &event); + + /* restart the driver */ + + if (engine->driver->start (engine->driver)) { + jack_error ("could not restart driver after freewheeling"); + return -1; + } + } - event.type = StopFreewheel; - jack_deliver_event_to_all (engine, &event); + return 0; +} - /* restart the driver */ +static int +jack_check_client_status (jack_engine_t* engine) +{ + JSList *node; + int err = 0; - if (engine->driver->start (engine->driver)) { - jack_error ("could not restart driver after freewheeling"); - return -1; + /* we are already late, or something else went wrong, + so it can't hurt to check the existence of all + clients. + */ + + for (node = engine->clients; node; + node = jack_slist_next (node)) { + jack_client_internal_t *client = + (jack_client_internal_t *) node->data; + + if (client->control->type == ClientExternal) { + if (kill (client->control->pid, 0)) { + VERBOSE(engine, + "client %s has died/exited", + client->control->name); + client->error++; + err++; + } + if(client->control->last_status != 0) { + VERBOSE(engine, + "client %s has nonzero process callback status (%d)\n", + client->control->name, client->control->last_status); + client->error++; + err++; + } + } + + DEBUG ("client %s errors = %d", client->control->name, + client->error); } - return 0; + + return err; } static int @@ -2103,7 +2178,8 @@ jack_run_one_cycle (jack_engine_t *engine, jack_nframes_t nframes, #define WORK_SCALE 1.0f - if (engine->control->real_time && + if (!engine->freewheeling && + engine->control->real_time && engine->spare_usecs && ((WORK_SCALE * engine->spare_usecs) <= delayed_usecs)) { @@ -2125,17 +2201,27 @@ jack_run_one_cycle (jack_engine_t *engine, jack_nframes_t nframes, consecutive_excessive_delays = 0; } - DEBUG ("trying to acquire read lock"); + DEBUG ("trying to acquire read lock (FW = %d)", engine->freewheeling); if (jack_try_rdlock_graph (engine)) { VERBOSE (engine, "lock-driven null cycle"); - driver->null_cycle (driver, nframes); + if (!engine->freewheeling) { + driver->null_cycle (driver, nframes); + } else { + /* don't return too fast */ + usleep (1000); + } return 0; } if (jack_trylock_problems (engine)) { VERBOSE (engine, "problem-lock-driven null cycle"); jack_unlock_graph (engine); - driver->null_cycle (driver, nframes); + if (!engine->freewheeling) { + driver->null_cycle (driver, nframes); + } else { + /* don't return too fast */ + usleep (1000); + } return 0; } @@ -2143,7 +2229,12 @@ jack_run_one_cycle (jack_engine_t *engine, jack_nframes_t nframes, VERBOSE (engine, "problem-driven null cycle problems=%d", engine->problems); jack_unlock_problems (engine); jack_unlock_graph (engine); - driver->null_cycle (driver, nframes); + if (!engine->freewheeling) { + driver->null_cycle (driver, nframes); + } else { + /* don't return too fast */ + usleep (1000); + } return 0; } @@ -2166,38 +2257,9 @@ jack_run_one_cycle (jack_engine_t *engine, jack_nframes_t nframes, } } else { - - JSList *node; DEBUG ("engine process cycle failed"); - - /* we are already late, or something else went wrong, - so it can't hurt to check the existence of all - clients. - */ - - for (node = engine->clients; node; - node = jack_slist_next (node)) { - jack_client_internal_t *client = - (jack_client_internal_t *) node->data; - - if (client->control->type == ClientExternal) { - if (kill (client->control->pid, 0)) { - VERBOSE(engine, - "client %s has died/exited", - client->control->name); - client->error++; - } - if(client->control->last_status != 0) { - VERBOSE(engine, - "client %s has nonzero process callback status (%d)\n", - client->control->name, client->control->last_status); - client->error++; - } - } - - DEBUG ("client %s errors = %d", client->control->name, - client->error); - } + jack_check_client_status (engine); + } jack_engine_post_process (engine); @@ -2311,6 +2373,8 @@ jack_engine_delete (jack_engine_t *engine) VERBOSE (engine, "starting server engine shutdown"); + jack_stop_freewheeling (engine, 1); + engine->control->engine_ok = 0; /* tell clients we're going away */ /* this will wake the server thread and cause it to exit */ @@ -2929,20 +2993,22 @@ jack_compute_all_port_total_latencies (jack_engine_t *engine) jack_port_shared_t *shared = engine->control->ports; unsigned int i; int toward_port; - + for (i = 0; i < engine->control->port_max; i++) { if (shared[i].in_use) { - if (shared[i].flags & JackPortIsOutput) { - toward_port = FALSE; - } else { - toward_port = TRUE; - } - shared[i].total_latency = - jack_get_port_total_latency ( - engine, &engine->internal_ports[i], - 0, toward_port); - } - } + + if (shared[i].flags & JackPortIsOutput) { + toward_port = FALSE; + } else { + toward_port = TRUE; + } + + shared[i].total_latency = + jack_get_port_total_latency ( + engine, &engine->internal_ports[i], + 0, toward_port); + } + } } /* How the sort works: diff --git a/libjack/client.c b/libjack/client.c index c9a5074..e4b40ec 100644 --- a/libjack/client.c +++ b/libjack/client.c @@ -1274,6 +1274,7 @@ jack_set_freewheel (jack_client_t* client, int onoff) jack_request_t request; request.type = onoff ? FreeWheel : StopFreeWheel; + request.x.client_id = client->control->id; return jack_client_deliver_request (client, &request); }