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

@@ -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

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__ */

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;
 		}
 		

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);

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:

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);
 }