new patch to help jackd along

Changes:

        * use poll+read, not just read, when waiting for clients to finish up
non-process "event" handling
        * mark clients as Finished after their process callback has
executed
        * remove clients that failed to respond to events
        * add new -r option to completely remove the JACK shm registry
at startup (orthogonal to everything else, but in my codebase for
months)

I would commit this directly, but I'm trying to be cautious for once. It
works much better for me now. Note that I believe there may be some
locking issues still to address in the code (insufficient locking, that
is, not deadlocks).

--p


[jack-20080508.patch]

---

Index: libjack/client.c
===================================================================
--- libjack/client.c (revision 1175)
+++ libjack/client.c (working copy)
@@ -1683,14 +1683,23 @@
  /* wait for first wakeup from server */
 
  if (jack_thread_first_wait (client) == control->nframes) {
+
  /* now run till we're done */
+
  if (control->process) {
+
  /* run process callback, then wait... ad-infinitum */
- while (jack_thread_wait (client,
- control->process (control->nframes,
-   control->process_arg)) ==
-       control->nframes)
- ;
+
+ while (1) {
+ int status = (control->process (control->nframes,
+ control->process_arg) ==
+      control->nframes);
+ control->state = Finished;
+ if (!jack_thread_wait (client, status)) {
+ break;
+ }
+ }
+
  } else {
  /* no process handling but still need to process events */
  while (jack_thread_wait (client, 0) == control->nframes)
Index: libjack/shm.c
===================================================================
--- libjack/shm.c (revision 1175)
+++ libjack/shm.c (working copy)
@@ -243,7 +243,7 @@
  * returns: 0 if successful
  */
 static int
-jack_server_initialize_shm (void)
+jack_server_initialize_shm (int new_registry)
 {
  int rc;
 
@@ -254,6 +254,11 @@
 
  rc = jack_access_registry (®istry_info);
 
+ if (new_registry) {
+ jack_remove_shm (®istry_id);
+ rc = ENOENT;
+ }
+
  switch (rc) {
  case ENOENT: /* registry does not exist */
  rc = jack_create_registry (®istry_info);
@@ -302,6 +307,7 @@
  jack_set_server_prefix (server_name);
 
  jack_shm_lock_registry ();
+
  if ((rc = jack_access_registry (®istry_info)) == 0) {
  if ((rc = jack_shm_validate_registry ()) != 0) {
  jack_error ("Incompatible shm registry, "
@@ -373,7 +379,7 @@
  *   ENOMEM if unable to access shared memory registry
  */
 int
-jack_register_server (const char *server_name)
+jack_register_server (const char *server_name, int new_registry)
 {
  int i;
  pid_t my_pid = getpid ();
@@ -382,7 +388,7 @@
 
  jack_info ("JACK compiled with %s SHM support.", JACK_SHM_TYPE);
 
- if (jack_server_initialize_shm ())
+ if (jack_server_initialize_shm (new_registry))
  return ENOMEM;
 
  jack_shm_lock_registry ();
Index: configure.ac
===================================================================
--- configure.ac (revision 1175)
+++ configure.ac (working copy)
@@ -789,5 +789,6 @@
 echo \| Shared memory interface............................... : $JACK_SHM_TYPE
 echo \| IPC Temporary directory............................... : $DEFAULT_TMP_DIR
 echo \| Install prefix........................................ : $prefix
+echo \| Default tmp dir....................................... : $DEFAULT_TMP_DIR
 echo
 
Index: jack/engine.h
===================================================================
--- jack/engine.h (revision 1175)
+++ jack/engine.h (working copy)
@@ -116,6 +116,7 @@
     int    reordered;
     int    watchdog_check;
     int    feedbackcount;
+    int             removing_clients;
     pid_t           wait_pid;
     pthread_t       freewheel_thread;
     int             nozombies;
Index: jack/shm.h
===================================================================
--- jack/shm.h (revision 1175)
+++ jack/shm.h (working copy)
@@ -95,7 +95,7 @@
 
 /* here beginneth the API */
 
-extern int  jack_register_server (const char *server_name);
+extern int  jack_register_server (const char *server_name, int new_registry);
 extern void jack_unregister_server (const char *server_name);
 
 extern int  jack_initialize_shm (const char *server_name);
Index: jackd/jackd.1.in
===================================================================
--- jackd/jackd.1.in (revision 1175)
+++ jackd/jackd.1.in (working copy)
@@ -58,6 +58,13 @@
 Set the maximum number of ports the JACK server can manage.  
 The default value is 256.
 .TP
+\fB\-r, \-\-replace-registry\fR
+.br
+Remove the shared memory registry used by all JACK server instances
+before startup. This should rarely be used, and is intended only
+for occasions when the structure of this registry changes in ways
+that are incompatible across JACK versions (which is rare).
+.TP
 \fB\-R, \-\-realtime\fR
 .br
 Use realtime scheduling.  This is needed for reliable low\-latency
Index: jackd/clientengine.c
===================================================================
--- jackd/clientengine.c (revision 1175)
+++ jackd/clientengine.c (working copy)
@@ -25,6 +25,7 @@
 
 #include <errno.h>
 #include <stdio.h>
+#include <unistd.h>
 #include <string.h>
 
 #include <jack/internal.h>
@@ -37,8 +38,6 @@
 #include "clientengine.h"
 #include "transengine.h"
 
-#define JACK_ERROR_WITH_SOCKETS 10000000
-
 static void
 jack_client_disconnect_ports (jack_engine_t *engine,
       jack_client_internal_t *client)
@@ -68,7 +67,10 @@
    jack_client_internal_t *client, int sort_graph)
 {
  /* caller must hold engine->client_lock and must have checked for and/or
- *   cleared all connections held by client. */
+ *   cleared all connections held by client.
+ */
+ VERBOSE(engine,"+++ deactivate %s", client->control->name);
+
  client->control->active = FALSE;
 
  jack_transport_client_exit (engine, client);
@@ -77,7 +79,7 @@
     engine->external_client_cnt > 0) {
  engine->external_client_cnt--;
  }
-
+
  if (sort_graph) {
  jack_sort_graph (engine);
  }
@@ -166,6 +168,12 @@
  JSList *tmp, *node;
  int need_sort = FALSE;
  jack_client_internal_t *client;
+
+ if (engine->removing_clients) {
+ return;
+ }
+
+ engine->removing_clients++;
 
  /* remove all dead clients */
 
@@ -174,6 +182,8 @@
  tmp = jack_slist_next (node);
 
  client = (jack_client_internal_t *) node->data;
+
+ VERBOSE(engine, "client %s error status %d", client->control->name, client->error);
 
  if (client->error) {
 
@@ -220,6 +230,8 @@
  }
 
  jack_engine_reset_rolling_usecs (engine);
+
+ engine->removing_clients--;
 }
 
 static int
@@ -848,6 +860,10 @@
 
  if (((jack_client_internal_t *) node->data)->request_fd == fd) {
  client = (jack_client_internal_t *) node->data;
+ VERBOSE (engine, "marking socket error on client %s state = "
+ "%s errors = %d", client->control->name,
+ jack_client_state_name (client),
+ client->error);
  if (client->error < JACK_ERROR_WITH_SOCKETS) {
  client->error += JACK_ERROR_WITH_SOCKETS;
  }
@@ -961,3 +977,4 @@
  req->status = JackFailure;
  }
 }
+
Index: jackd/engine.c
===================================================================
--- jackd/engine.c (revision 1175)
+++ jackd/engine.c (working copy)
@@ -1018,8 +1018,9 @@
 void
 jack_driver_unload (jack_driver_t *driver)
 {
+ void* handle = driver->handle;
  driver->finish (driver);
- dlclose (driver->handle);
+ dlclose (handle);
 }
 
 int
@@ -1471,6 +1472,7 @@
  }
 
  if (pfd[i].revents & ~POLLIN) {
+ VERBOSE (engine, "client poll reports non-input condition, fd was %d", pfd[i].fd);
  jack_client_disconnect (engine, pfd[i].fd);
  } else if (pfd[i].revents & POLLIN) {
  if (handle_external_client_request
@@ -1630,6 +1632,7 @@
  engine->feedbackcount = 0;
  engine->wait_pid = wait_pid;
  engine->nozombies = nozombies;
+ engine->removing_clients = 0;
 
  engine->audio_out_cnt = 0;
  engine->audio_in_cnt = 0;
@@ -1815,7 +1818,6 @@
 static void
 jack_engine_delay (jack_engine_t *engine, float delayed_usecs)
 {
- JSList *node;
  jack_event_t event;
 
  engine->control->frame_timer.reset_pending = 1;
@@ -1827,13 +1829,7 @@
 
  event.type = XRun;
 
- jack_lock_graph (engine);
- for (node = engine->clients; node; node = jack_slist_next (node)) {
- jack_deliver_event (engine,
-    (jack_client_internal_t *) node->data,
-    &event);
- }
- jack_unlock_graph (engine);
+ jack_deliver_event_to_all (engine, &event);
 }
 
 static inline void
@@ -2265,6 +2261,7 @@
     event);
  }
  jack_unlock_graph (engine);
+ jack_remove_clients (engine);
 }
 
 static void
@@ -2307,9 +2304,8 @@
    our check on a client's continued well-being
  */
 
- if (client->control->dead
-    || (client->control->type == ClientExternal
- && kill (client->control->pid, 0))) {
+ if (client->control->dead || client->error >= JACK_ERROR_WITH_SOCKETS
+    || (client->control->type == ClientExternal && kill (client->control->pid, 0))) {
  DEBUG ("client %s is dead - no event sent",
        client->control->name);
  return 0;
@@ -2378,32 +2374,74 @@
  jack_error ("cannot send event to client [%s]"
     " (%s)", client->control->name,
     strerror (errno));
- client->error++;
+ client->error = JACK_ERROR_WITH_SOCKETS+99;
  }
-
- DEBUG ("engine reading from event fd");
-
- if (!client->error &&
-    (read (client->event_fd, &status, sizeof (status))
-     != sizeof (status))) {
- jack_error ("cannot read event response from "
-    "client [%s] (%s)",
-    client->control->name,
-    strerror (errno));
- client->error++;
- }
 
- DEBUG ("engine reading from event fd DONE");
-
- if (status != 0) {
- jack_error ("bad status for client event "
-    "handling (type = %d)",
-    event->type);
- client->error++;
- }
+ if (client->error) {
+ status = 1;
+ } else {
+ // then we check whether there really is an error.... :)
+
+ struct pollfd pfd[1];
+ pfd[0].fd = client->event_fd;
+ //pfd[0].events = POLLERR|POLLIN|POLLHUP|POLLNVAL;
+ pfd[0].events = POLLIN;
+
+ int poll_timeout = (engine->client_timeout_msecs > 0 ?
+ engine->client_timeout_msecs :
+ 1 + engine->driver->period_usecs/1000);
+
+ //poll_timeout = 200;
+ //poll_timeout = 30000; // 30 seconds
+
+ int poll_ret;
+ // printf("################ poll_timeout = %d (%d or 1 + %d/1000)\n", poll_timeout, engine->client_timeout_msecs, engine->driver->period_usecs);
+
+ if ( (poll_ret = poll (pfd, 1, poll_timeout)) < 0) {
+ DEBUG ("client event poll not ok! (-1) poll returned an error");
+ jack_error ("poll on subgraph processing failed (%s)", strerror (errno));
+ status = -1;
+ } else {
+
+ DEBUG ("\n\n\n\n\n back from client event poll, revents = 0x%x\n\n\n", pfd[0].revents);
+
+ if (pfd[0].revents & ~POLLIN) {
+ DEBUG ("client event poll not ok! (-2), revents = %d\n", pfd[0].revents);
+ jack_error ("subgraph starting at %s lost client", client->control->name);
+ status = -2;
+ }
+
+ if (pfd[0].revents & POLLIN) {
+ DEBUG ("client event poll ok!");
+ status = 0;
+ } else {
+ DEBUG ("client event poll not ok! (1 = poll timed out, revents = 0x%04x, poll_ret = %d)", pfd[0].revents, poll_ret);
+ jack_error ("subgraph starting at %s timed out "
+ "(subgraph_wait_fd=%d, status = %d, state = %s, revents = 0x%04x)",
+ client->control->name,
+ client->subgraph_wait_fd, status,
+ jack_client_state_name (client), pfd[0].revents);
+ status = 1;
+ }
+ }
+   }
+
+ if (status == 0) {
+ if (read (client->event_fd, &status, sizeof (status)) != sizeof (status)) {
+ jack_error ("cannot read event response from "
+ "client [%s] (%s)",
+ client->control->name,
+ strerror (errno));
+ client->error = JACK_ERROR_WITH_SOCKETS+99;
+ }
+ } else {
+ jack_error ("bad status (%d) for client event "
+      "handling (type = %d)",
+    status,event->type);
+ client->error = JACK_ERROR_WITH_SOCKETS+99;
+   }
  }
  }
-
  DEBUG ("event delivered");
 
  return 0;
@@ -2431,6 +2469,10 @@
 
  next = jack_slist_next (node);
 
+ 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 = (jack_client_internal_t *) node->data;
@@ -2560,6 +2602,8 @@
  subgraph_client->subgraph_wait_fd, n);
  }
 
+ jack_remove_clients (engine);
+
  VERBOSE (engine, "-- jack_rechain_graph()");
 
  return err;
@@ -3161,6 +3205,8 @@
  }
 
  jack_unlock_graph (engine);
+ jack_remove_clients (engine);
+
  return 0;
 }
 
@@ -3262,6 +3308,8 @@
  }
  }
 
+ jack_remove_clients (engine);
+
  if (check_acyclic) {
  jack_check_acyclic (engine);
  }
@@ -3785,6 +3833,8 @@
  }
  }
  }
+
+ jack_remove_clients (engine);
 }
 
 void
@@ -3820,6 +3870,8 @@
  }
  }
  }
+
+ jack_remove_clients (engine);
 }
 
 int
Index: jackd/clientengine.h
===================================================================
--- jackd/clientengine.h (revision 1175)
+++ jackd/clientengine.h (working copy)
@@ -40,6 +40,8 @@
  return client_state_names[client->control->state];
 }
 
+#define JACK_ERROR_WITH_SOCKETS 10000000
+
 int jack_client_activate (jack_engine_t *engine, jack_client_id_t id);
 int jack_client_deactivate (jack_engine_t *engine, jack_client_id_t id);
 int jack_client_create (jack_engine_t *engine, int client_fd);
Index: jackd/jackd.c
===================================================================
--- jackd/jackd.c (revision 1175)
+++ jackd/jackd.c (working copy)
@@ -371,6 +371,7 @@
 "             [ --debug-timer OR -D ]\n"
 "             [ --verbose OR -v ]\n"
 "             [ --clocksource OR -c [ c(ycle) | h(pet) | s(ystem) ]\n"
+"             [ --replace-registry OR -r ]\n"
 "             [ --silent OR -s ]\n"
 "             [ --version OR -V ]\n"
 "             [ --nozombies OR -Z ]\n"
@@ -520,6 +521,7 @@
  { "name", 1, 0, 'n' },
  { "unlock", 0, 0, 'u' },
  { "realtime", 0, 0, 'R' },
+ { "replace-registry", 0, 0, 'r' },
  { "realtime-priority", 1, 0, 'P' },
  { "timeout", 1, 0, 't' },
  { "temporary", 0, 0, 'T' },
@@ -537,6 +539,7 @@
  JSList * driver_params;
  int driver_nargs = 1;
  int show_version = 0;
+ int replace_registry = 0;
  int i;
  int rc;
 
@@ -593,6 +596,10 @@
  realtime_priority = atoi (optarg);
  break;
 
+ case 'r':
+ replace_registry = 1;
+ break;
+
  case 'R':
  realtime = 1;
  break;
@@ -695,7 +702,7 @@
 
  copyright (stdout);
 
- rc = jack_register_server (server_name);
+ rc = jack_register_server (server_name, replace_registry);
  switch (rc) {
  case EEXIST:
  fprintf (stderr, "`%s' server already active\n", server_name);

---

_______________________________________________
Jack-Devel mailing list
Jack-Devel@...
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org