cleanup shm, set temp dir, improve overall thread/child/parent relationships in jackd

git-svn-id: svn+ssh://jackaudio.org/trunk/jack@79 0c269be4-1314-0410-8aa9-9f06e86f4224

alsa_driver.c

@@ -716,7 +716,7 @@ alsa_driver_wait (alsa_driver_t *driver)
 	while (need_playback || need_capture) {
 
 		int p_timed_out, c_timed_out;
-		int ci;
+		int ci = 0;
 		int nfds;
 
 		nfds = 0;

client.c

@@ -37,6 +37,14 @@
 #include <jack/pool.h>
 #include <jack/error.h>
 
+char *jack_temp_dir = "/tmp";
+
+void
+jack_set_temp_dir (const char *path)
+{
+	jack_temp_dir = strdup (path);
+}
+
 static jack_port_t *jack_port_new (jack_client_t *client, jack_port_id_t port_id, jack_control_t *control);
 
 static pthread_mutex_t client_lock;
@@ -250,7 +258,7 @@ server_connect (int which)
 	}
 
 	addr.sun_family = AF_UNIX;
-	g_snprintf (addr.sun_path, sizeof (addr.sun_path) - 1, "/tmp/jack_%d", which);
+	g_snprintf (addr.sun_path, sizeof (addr.sun_path) - 1, "%s/jack_%d", jack_temp_dir, which);
 
 	if (connect (fd, (struct sockaddr *) &addr, sizeof (addr)) < 0) {
 		jack_error ("cannot connect to jack server", strerror (errno));
@@ -276,7 +284,7 @@ server_event_connect (jack_client_t *client)
 	}
 
 	addr.sun_family = AF_UNIX;
-	g_snprintf (addr.sun_path, sizeof (addr.sun_path) - 1, "/tmp/jack_ack_0");
+	g_snprintf (addr.sun_path, sizeof (addr.sun_path) - 1, "%s/jack_ack_0", jack_temp_dir);
 
 	if (connect (fd, (struct sockaddr *) &addr, sizeof (addr)) < 0) {
 		jack_error ("cannot connect to jack server for events", strerror (errno));

configure.in

@@ -4,8 +4,8 @@ AC_INIT(client.c)
 AC_CONFIG_AUX_DIR(.)
 
 JACK_MAJOR_VERSION=0
-JACK_MINOR_VERSION=4
-JACK_MICRO_VERSION=7
+JACK_MINOR_VERSION=5
+JACK_MICRO_VERSION=0
 
 BETA=
 

engine.c

@@ -28,6 +28,7 @@
 #include <sys/shm.h>
 #include <stdio.h>
 #include <stdarg.h>
+#include <dirent.h>
 #include <sys/ipc.h>
 #include <signal.h>
 #include <sys/types.h>
@@ -41,6 +42,8 @@
 #include <jack/engine.h>
 #include <jack/driver.h>
 
+#define MAX_SHM_ID 256 /* likely use is more like 16 */
+
 typedef struct {
 
     jack_port_internal_t *source;
@@ -94,6 +97,8 @@ static int  jack_deliver_event (jack_engine_t *, jack_client_internal_t *, jack_
 
 static void jack_audio_port_mixdown (jack_port_t *port, nframes_t nframes);
 
+static int *jack_shm_registry;
+static int  jack_shm_id_cnt;
 
 jack_port_type_info_t builtin_port_types[] = {
 	{ JACK_DEFAULT_AUDIO_TYPE, jack_audio_port_mixdown, 1 },
@@ -106,14 +111,6 @@ jack_client_is_inprocess (jack_client_internal_t *client)
 	return (client->control->type == ClientDynamic) || (client->control->type == ClientDriver);
 }
 
-static
-void shm_destroy (int status, void *arg)
-
-{
-	int shm_id = (int) arg;
-	shmctl (shm_id, IPC_RMID, 0);
-}
-
 static int
 make_sockets (int fd[2])
 {
@@ -129,7 +126,7 @@ make_sockets (int fd[2])
 
 	addr.sun_family = AF_UNIX;
 	for (i = 0; i < 999; i++) {
-		snprintf (addr.sun_path, sizeof (addr.sun_path) - 1, "/tmp/jack_%d", i);
+		snprintf (addr.sun_path, sizeof (addr.sun_path) - 1, "%s/jack_%d", jack_temp_dir, i);
 		if (access (addr.sun_path, F_OK) != 0) {
 			break;
 		}
@@ -163,7 +160,7 @@ make_sockets (int fd[2])
 
 	addr.sun_family = AF_UNIX;
 	for (i = 0; i < 999; i++) {
-		snprintf (addr.sun_path, sizeof (addr.sun_path) - 1, "/tmp/jack_ack_%d", i);
+		snprintf (addr.sun_path, sizeof (addr.sun_path) - 1, "%s/jack_ack_%d", jack_temp_dir, i);
 		if (access (addr.sun_path, F_OK) != 0) {
 			break;
 		}
@@ -229,6 +226,88 @@ jack_cleanup_clients (jack_engine_t *engine)
 	}
 }	
 
+static int
+jack_initialize_shm ()
+{
+	int shmid_id;
+	void *addr;
+
+	if (jack_shm_registry != NULL) {
+		return 0;
+	}
+
+	/* grab a chunk of memory to store shm ids in. this is 
+	   to allow our parent to clean up all such ids when
+	   if we exit. otherwise, they can get lost in crash
+	   or debugger driven exits.
+	*/
+	
+	if ((shmid_id = shmget (random(), sizeof(int) * MAX_SHM_ID, IPC_CREAT|0600)) < 0) {
+		jack_error ("cannot create engine shm ID registry (%s)", strerror (errno));
+		return -1;
+	}
+	if ((addr = shmat (shmid_id, 0, 0)) == (void *) -1) {
+		jack_error ("cannot attach shm ID registry (%s)", strerror (errno));
+		shmctl (shmid_id, IPC_RMID, 0);
+		return -1;
+	}
+	if (shmctl (shmid_id, IPC_RMID, NULL)) {
+		jack_error ("cannot mark shm ID registry as destroyed (%s)", strerror (errno));
+		return -1;
+	}
+
+	jack_shm_registry = (int *) addr;
+	jack_shm_id_cnt = 0;
+
+	return 0;
+}
+
+static void
+jack_register_shm (int shmid)
+{
+	if (jack_shm_id_cnt < MAX_SHM_ID) {
+		jack_shm_registry[jack_shm_id_cnt++] = shmid;
+	}
+}
+
+void
+jack_cleanup_shm ()
+{
+	int i;
+
+	for (i = 0; i < jack_shm_id_cnt; i++) {
+		fprintf (stderr, "removing shm ID[%d] = %d\n", i, jack_shm_registry[i]);
+		shmctl (jack_shm_registry[i], IPC_RMID, NULL);
+	}
+}
+
+void
+jack_cleanup_files ()
+{
+	DIR *dir;
+	struct dirent *dirent;
+
+	/* its important that we remove all files that jackd creates
+	   because otherwise subsequent attempts to start jackd will
+	   believe that an instance is already running.
+	*/
+
+	if ((dir = opendir (jack_temp_dir)) == NULL) {
+		fprintf (stderr, "jack(%d): cannot open jack FIFO directory (%s)\n", getpid(), strerror (errno));
+		return;
+	}
+
+	while ((dirent = readdir (dir)) != NULL) {
+		if (strncmp (dirent->d_name, "jack-", 5) == 0 || strncmp (dirent->d_name, "jack_", 5) == 0) {
+			char fullpath[PATH_MAX+1];
+			sprintf (fullpath, "%s/%s", jack_temp_dir, dirent->d_name);
+			unlink (fullpath);
+		}
+	}
+
+	closedir (dir);
+}
+
 static int
 jack_add_port_segment (jack_engine_t *engine, unsigned long nports)
 
@@ -249,14 +328,14 @@ jack_add_port_segment (jack_engine_t *engine, unsigned long nports)
 		return -1;
 	}
 	
+	jack_register_shm (id);
+
 	if ((addr = shmat (id, 0, 0)) == (char *) -1) {
 		jack_error ("cannot attach new port segment (%s)", strerror (errno));
 		shmctl (id, IPC_RMID, 0);
 		return -1;
 	}
 		
-	on_exit (shm_destroy, (void *) id);
-		
 	si = (jack_port_segment_info_t *) malloc (sizeof (jack_port_segment_info_t));
 	si->shm_key = key;
 	si->address = addr;
@@ -1026,7 +1105,6 @@ jack_engine_new (int realtime, int rtpriority)
 	engine->port_max = 128;
 	engine->rtpriority = rtpriority;
 	engine->silent_buffer = 0;
-	engine->getthehelloutathere = FALSE;
 
 	pthread_mutex_init (&engine->graph_lock, 0);
 	pthread_mutex_init (&engine->buffer_lock, 0);
@@ -1064,10 +1142,16 @@ jack_engine_new (int realtime, int rtpriority)
 	engine->control_key = random();
 	control_size = sizeof (jack_control_t) + (sizeof (jack_port_shared_t) * engine->port_max);
 
+	if (jack_initialize_shm (engine)) {
+		return 0;
+	}
+
 	if ((engine->control_shm_id = shmget (engine->control_key, control_size, IPC_CREAT|0644)) < 0) {
 		jack_error ("cannot create engine control shared memory segment (%s)", strerror (errno));
 		return 0;
 	}
+
+	jack_register_shm (engine->control_shm_id);
 	
 	if ((addr = shmat (engine->control_shm_id, 0, 0)) == (void *) -1) {
 		jack_error ("cannot attach control shared memory segment (%s)", strerror (errno));
@@ -1075,8 +1159,6 @@ jack_engine_new (int realtime, int rtpriority)
 		return 0;
 	}
 
-	on_exit (shm_destroy, (void *) engine->control_shm_id);
-
 	engine->control = (jack_control_t *) addr;
 
 	/* Mark all ports as available */
@@ -1109,7 +1191,7 @@ jack_engine_new (int realtime, int rtpriority)
 	engine->control->buffer_size = 0;
 	engine->control->frame_time = 0;
 
-	sprintf (engine->fifo_prefix, "/tmp/jack_fifo_%d", getpid());
+	sprintf (engine->fifo_prefix, "%s/ack_fifo_%d", jack_temp_dir, getpid());
 
 	(void) jack_get_fifo_fd (engine, 0);
 	jack_start_server (engine);
@@ -1227,7 +1309,9 @@ int
 jack_engine_delete (jack_engine_t *engine)
 
 {
-	pthread_cancel (engine->main_thread);
+	if (engine) {
+		return pthread_cancel (engine->main_thread);
+	}
 	return 0;
 }
 
@@ -1253,6 +1337,8 @@ jack_client_internal_new (jack_engine_t *engine, int fd, jack_client_connect_req
 			jack_error ("cannot create client control block");
 			return 0;
 		}
+		
+		jack_register_shm (shm_id);
 
 		if ((addr = shmat (shm_id, 0, 0)) == (void *) -1) {
 			jack_error ("cannot attach new client control block");

jack/engine.h

@@ -73,15 +73,15 @@ struct _jack_engine {
     unsigned long fifo_size;
     unsigned long external_client_cnt;
     int rtpriority;
-    int getthehelloutathere;
 };
 
 /* public functions */
 
-jack_engine_t *jack_engine_new (int real_time, int real_time_priority);
+jack_engine_t  *jack_engine_new (int real_time, int real_time_priority);
 int             jack_engine_delete (jack_engine_t *);
 int             jack_run (jack_engine_t *engine);
 int             jack_wait (jack_engine_t *engine);
 int             jack_use_driver (jack_engine_t *, struct _jack_driver *);
+void            jack_set_temp_dir (const char *);
 
 #endif /* __jack_engine_h__ */

jack/internal.h

@@ -203,10 +203,15 @@ typedef struct {
     int status;
 }  jack_request_t;
 
+extern void jack_cleanup_shm ();
+extern void jack_cleanup_files ();
+
 extern int jack_client_handle_port_connection (jack_client_t *client, jack_event_t *event);
 
 jack_client_t *jack_driver_become_client (const char *client_name);
 
+extern char *jack_temp_dir;
+
 #endif /* __jack_internal_h__ */
 
 

jackd.c

@@ -22,7 +22,7 @@
 #include <signal.h>
 #include <getopt.h>
 #include <sys/types.h>
-#include <dirent.h>
+#include <sys/shm.h>
 #include <string.h>
 #include <errno.h>
 #include <wait.h>
@@ -32,7 +32,7 @@
 #include <jack/driver.h>
 
 static sigset_t signals;
-static jack_engine_t *engine;
+static jack_engine_t *engine = 0;
 static int jackd_pid;
 static char *alsa_pcm_name = "default";
 static nframes_t frames_per_interrupt = 64;
@@ -40,111 +40,18 @@ static nframes_t srate = 48000;
 static int realtime = 0;
 static int realtime_priority = 10;
 static int with_fork = 1;
-static int need_cleanup = 1;
-
-#define JACK_TEMP_DIR "/tmp"
-
-static void
-cleanup ()
-{
-	DIR *dir;
-	struct dirent *dirent;
-
-	/* this doesn't have to truly atomic. in fact, its not even strictly
-	   necessary. it just potentially saves us from thrashing through
-	   the temp dir several times over.
-	*/
-
-	if (!need_cleanup) {
-		return;
-	}
-
-	need_cleanup = 0;
-
-	/* its important that we remove all files that jackd creates
-	   because otherwise subsequent attempts to start jackd will
-	   believe that an instance is already running.
-	*/
-
-	if ((dir = opendir (JACK_TEMP_DIR)) == NULL) {
-		fprintf (stderr, "jackd(%d): cleanup - cannot open scratch directory (%s)\n", getpid(), strerror (errno));
-		return;
-	}
-
-	while ((dirent = readdir (dir)) != NULL) {
-		if (strncmp (dirent->d_name, "jack-", 5) == 0 || strncmp (dirent->d_name, "jack_", 5) == 0) {
-			char fullpath[PATH_MAX+1];
-			sprintf (fullpath, JACK_TEMP_DIR "/%s", dirent->d_name);
-			unlink (fullpath);
-		}
-	}
-
-	closedir (dir);
-}
 
 static void
 signal_handler (int sig)
 {
-	fprintf (stderr, "parent (%d): killing jackd at %d\n", getpid(), jackd_pid);
+	fprintf (stderr, "jackd: signal %d received\n", sig);
 	kill (jackd_pid, SIGTERM);
-	cleanup ();
-	exit (1);
 }
 
 static void
-catch_signals (void)
-{
-	/* what's this for? 
-
-	   this just makes sure that if we are using the fork
-	   approach to cleanup (see main()), the waiting
-	   process will catch common "interrupt" signals
-	   and terminate the real server appropriately.
-	*/
-
-	signal (SIGHUP, signal_handler);
-	signal (SIGINT, signal_handler);
-	signal (SIGQUIT, signal_handler);
-	signal (SIGTERM, signal_handler);
-}
-
-static void *
-signal_thread (void *arg)
-
-{
-	int sig;
-	int err;
-
-	pthread_setcanceltype (PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-	
-	/* Note: normal operation has with_form == 1 */
-
-	if (with_fork) {
-		/* let the parent handle SIGINT */
-		sigdelset (&signals, SIGINT);
-	}
-
-	err = sigwait (&signals, &sig);
-	fprintf (stderr, "child (%d): exiting due to signal %d\n", getpid(), sig);
-	jack_engine_delete (engine);
-
-	if (!with_fork) {
-		/* no parent - take care of this ourselves */
-		cleanup ();
-	}
-
-	exit (err);
-
-	/*NOTREACHED*/
-	return 0;
-}
-
-static int
 posix_me_harder (void)
 
 {
-	pthread_t thread_id;
-
 	/* what's this for?
 
 	   POSIX says that signals are delivered like this:
@@ -156,11 +63,11 @@ posix_me_harder (void)
 
            this means that a simple-minded multi-threaded
 	   program can expect to get POSIX signals delivered
-	   to any of its threads.
+	   randomly to any one of its threads, 
 
 	   here, we block all signals that we think we
-	   might receive and want to catch. all later
-	   threads will inherit this setting. then we
+	   might receive and want to catch. all "child"
+	   threads will inherit this setting. if we
 	   create a thread that calls sigwait() on the
 	   same set of signals, implicitly unblocking
 	   all those signals. any of those signals that
@@ -187,7 +94,7 @@ posix_me_harder (void)
 	sigaddset(&signals, SIGTERM);
 
 	/* this can make debugging a pain, but it also makes
-	   segv-exits cleanup after themselves rather than
+	   segv-exits cleanup_files after themselves rather than
 	   leaving the audio thread active. i still
 	   find it truly wierd that _exit() or whatever is done
 	   by the default SIGSEGV handler does not
@@ -200,44 +107,92 @@ posix_me_harder (void)
 	/* all child threads will inherit this mask */
 
 	pthread_sigmask (SIG_BLOCK, &signals, 0);
-
-	/* start a thread to wait for signals */
-
-	if (pthread_create (&thread_id, 0, signal_thread, 0)) {
-		fprintf (stderr, "cannot create signal catching thread");
-		return -1;
-	}
-
-	pthread_detach (thread_id);
-
-	return 0;
 }
 
-static void
-jack_main ()
+static void *
+jack_engine_waiter_thread (void *arg)
 {
+	pid_t signal_pid = (pid_t) arg;
 	jack_driver_t *driver;
 
-	posix_me_harder ();
+	pthread_setcanceltype (PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
 
 	if ((engine = jack_engine_new (realtime, realtime_priority)) == 0) {
 		fprintf (stderr, "cannot create engine\n");
-		return;
+		kill (signal_pid, SIGTERM);
+		return 0;
 	}
 
 	if ((driver = jack_driver_load (ADDON_DIR "/jack_alsa.so", alsa_pcm_name, frames_per_interrupt, srate)) == 0) {
 		fprintf (stderr, "cannot load ALSA driver module\n");
-		return;
+		kill (signal_pid, SIGTERM);
+		return 0;
 	}
 
 	jack_use_driver (engine, driver);
 
 	if (jack_run (engine)) {
 		fprintf (stderr, "cannot start main JACK thread\n");
-		return;
+		kill (signal_pid, SIGTERM);
+		return 0;
 	}
 
 	jack_wait (engine);
+
+	fprintf (stderr, "telling signal thread that the engine is done\n");
+	kill (signal_pid, SIGHUP);
+
+	return 0; /* nobody cares what this returns */
+}
+
+static void
+jack_main ()
+{
+	int sig;
+	int err;
+	pthread_t waiter_thread;
+
+	pthread_setcanceltype (PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
+
+	posix_me_harder ();
+	
+	/* what we'd really like to do here is to be able to 
+	   wait for either the engine to stop or a POSIX signal,
+	   whichever arrives sooner. but there is no mechanism
+	   to do that, so instead we create a thread to wait
+	   for the engine to finish, and here we stop and wait
+	   for any (reasonably likely) POSIX signal.
+
+	   if the engine finishes first, the waiter thread will
+	   tell us about it via a signal.
+
+	   if a signal arrives, we'll stop the engine and then
+	   exit. 
+
+	   in normal operation, our parent process will be waiting
+	   for us and will cleanup.
+	*/
+
+	if (pthread_create (&waiter_thread, 0, jack_engine_waiter_thread, (void *) getpid())) {
+		fprintf (stderr, "jackd: cannot create engine waiting thread\n");
+		return;
+	}
+
+	/* Note: normal operation has with_fork == 1 */
+
+	if (with_fork) {
+		/* let the parent handle SIGINT */
+		sigdelset (&signals, SIGINT);
+	}
+
+	err = sigwait (&signals, &sig);
+
+	fprintf (stderr, "signal waiter: exiting due to signal %d\n", sig);
+
+	pthread_cancel (waiter_thread);
+	jack_engine_delete (engine);
+
+	return;
 }
 
 static void usage () 
@@ -256,9 +211,10 @@ int
 main (int argc, char *argv[])
 
 {
-	const char *options = "hd:r:p:RP:F";
+	const char *options = "hd:r:p:RP:FD:";
 	struct option long_options[] = 
 	{ 
+		{ "tmpdir", 1, 0, 'D' },
 		{ "device", 1, 0, 'd' },
 		{ "srate", 1, 0, 'r' },
 		{ "frames-per-interrupt", 1, 0, 'p' },
@@ -274,6 +230,10 @@ main (int argc, char *argv[])
 	opterr = 0;
 	while ((opt = getopt_long (argc, argv, options, long_options, &option_index)) != EOF) {
 		switch (opt) {
+		case 'D':
+			jack_set_temp_dir (optarg);
+			break;
+
 		case 'd':
 			alsa_pcm_name = optarg;
 			break;
@@ -307,26 +267,39 @@ main (int argc, char *argv[])
 	}
 
 	if (!with_fork) {
+
+		/* This is really here so that we can run gdb easily */
+
 		jack_main ();
-		cleanup ();
 
 	} else {
 
 		int pid = fork ();
 		
 		if (pid < 0) {
+
 			fprintf (stderr, "could not fork jack server (%s)", strerror (errno));
 			exit (1);
+
 		} else if (pid == 0) {
+
 			jack_main ();
+
 		} else {
 			jackd_pid = pid;
-			catch_signals ();
+
+			signal (SIGHUP, signal_handler);
+			signal (SIGINT, signal_handler);
+			signal (SIGQUIT, signal_handler);
+			signal (SIGTERM, signal_handler);
+
 			waitpid (pid, NULL, 0);
-			cleanup ();
 		}
 	}
 
+	jack_cleanup_shm ();
+	jack_cleanup_files ();
+
 	return 0;
 }