Simplify the way jobs work and drop the persist type, so all jobs are

fire-and-forget.

Status jobs now managed with two trees of output (new and old), rather
than storing the output in the jobs themselves. When the status line is
processed any jobs which don't appear in the new tree are started and
the output from the old tree displayed. When a job finishes it updates
the new tree with its output and that is used for any subsequent
redraws. When the status interval expires, the new tree is moved to the
old so that all jobs are run again.

This fixes the "#(echo %H:%M:%S)" problem which would lead to thousands
of identical persistent jobs and high memory use (this can still be
achieved by adding "sleep 30" but that is much less likely to happen by
accident).

cmd-if-shell.c

@@ -53,7 +53,7 @@ cmd_if_shell_exec(struct cmd *self, struct cmd_ctx *ctx)
 {
 	struct args			*args = self->args;
 	struct cmd_if_shell_data	*cdata;
-	struct job			*job;
+	const char			*shellcmd = args->argv[0];
 
 	cdata = xmalloc(sizeof *cdata);
 	cdata->cmd = xstrdup(args->argv[1]);
@@ -64,9 +64,7 @@ cmd_if_shell_exec(struct cmd *self, struct cmd_ctx *ctx)
 	if (ctx->curclient != NULL)
 		ctx->curclient->references++;
 
-	job = job_add(NULL, 0, NULL,
+	job_run(shellcmd, cmd_if_shell_callback, cmd_if_shell_free, cdata);
-	    args->argv[0], cmd_if_shell_callback, cmd_if_shell_free, cdata);
-	job_run(job);
 
 	return (1);	/* don't let client exit */
 }

cmd-run-shell.c

@@ -53,7 +53,7 @@ cmd_run_shell_exec(struct cmd *self, struct cmd_ctx *ctx)
 {
 	struct args			*args = self->args;
 	struct cmd_run_shell_data	*cdata;
-	struct job			*job;
+	const char			*shellcmd = args->argv[0];
 
 	cdata = xmalloc(sizeof *cdata);
 	cdata->cmd = xstrdup(args->argv[0]);
@@ -64,9 +64,7 @@ cmd_run_shell_exec(struct cmd *self, struct cmd_ctx *ctx)
 	if (ctx->curclient != NULL)
 		ctx->curclient->references++;
 
-	job = job_add(NULL, 0, NULL,
+	job_run(shellcmd, cmd_run_shell_callback, cmd_run_shell_free, cdata);
-	    args->argv[0], cmd_run_shell_callback, cmd_run_shell_free, cdata);
-	job_run(job);
 
 	return (1);	/* don't let client exit */
 }

cmd-server-info.c

@@ -175,8 +175,8 @@ cmd_server_info_exec(unused struct cmd *self, struct cmd_ctx *ctx)
 
 	ctx->print(ctx, "Jobs:");
 	LIST_FOREACH(job, &all_jobs, lentry) {
-		ctx->print(ctx, "%s [fd=%d, pid=%d, status=%d, flags=0x%x]",
+		ctx->print(ctx, "%s [fd=%d, pid=%d, status=%d]",
-		    job->cmd, job->fd, job->pid, job->status, job->flags);
+		    job->cmd, job->fd, job->pid, job->status);
 	}
 
 	return (0);

cmd-set-option.c

@@ -87,11 +87,8 @@ cmd_set_option_exec(struct cmd *self, struct cmd_ctx *ctx)
 	struct winlink				*wl;
 	struct client				*c;
 	struct options				*oo;
-	struct jobs				*jobs;
-	struct job				*job, *nextjob;
 	const char				*optstr, *valstr;
 	u_int					 i;
-	int					 try_again;
 
 	/* Work out the options tree and table to use. */
 	if (args_has(self->args, 's')) {
@@ -181,24 +178,8 @@ cmd_set_option_exec(struct cmd *self, struct cmd_ctx *ctx)
 	    strcmp(oe->name, "window-status-format") == 0) {
 		for (i = 0; i < ARRAY_LENGTH(&clients); i++) {
 			c = ARRAY_ITEM(&clients, i);
-			if (c == NULL || c->session == NULL)
+			if (c != NULL && c->session != NULL)
-				continue;
+				server_redraw_client(c);
-
-			jobs = &c->status_jobs;
-			do {
-				try_again = 0;
-				job = RB_ROOT(jobs);
-				while (job != NULL) {
-					nextjob = RB_NEXT(jobs, jobs, job);
-					if (job->flags & JOB_PERSIST) {
-						job_remove(jobs, job);
-						try_again = 1;
-						break;
-					}
-					job = nextjob;
-				}
-			} while (try_again);
-			server_redraw_client(c);
 		}
 	}
 

job.c

@@ -31,129 +31,32 @@
  * output.
  */
 
+void	job_callback(struct bufferevent *, short, void *);
+
 /* All jobs list. */
 struct joblist	all_jobs = LIST_HEAD_INITIALIZER(all_jobs);
 
-RB_GENERATE(jobs, job, entry, job_cmp);
+/* Start a job running, if it isn't already. */
-
-void	job_callback(struct bufferevent *, short, void *);
-
-int
-job_cmp(struct job *job1, struct job *job2)
-{
-	return (strcmp(job1->cmd, job2->cmd));
-}
-
-/* Initialise job tree. */
-void
-job_tree_init(struct jobs *jobs)
-{
-	RB_INIT(jobs);
-}
-
-/* Destroy a job tree. */
-void
-job_tree_free(struct jobs *jobs)
-{
-	struct job	*job;
-
-	while (!RB_EMPTY(jobs)) {
-		job = RB_ROOT(jobs);
-		RB_REMOVE(jobs, jobs, job);
-		job_free(job);
-	}
-}
-
-/* Find a job and return it. */
 struct job *
-job_get(struct jobs *jobs, const char *cmd)
+job_run(const char *cmd,
-{
-	struct job	job;
-
-	job.cmd = (char *) cmd;
-	return (RB_FIND(jobs, jobs, &job));
-}
-
-/* Add a job. */
-struct job *
-job_add(struct jobs *jobs, int flags, struct client *c, const char *cmd,
     void (*callbackfn)(struct job *), void (*freefn)(void *), void *data)
 {
 	struct job	*job;
-
+	struct environ	 env;
-	job = xmalloc(sizeof *job);
+	pid_t		 pid;
-	job->cmd = xstrdup(cmd);
+	int		 nullfd, out[2];
-	job->pid = -1;
-	job->status = 0;
-
-	job->client = c;
-
-	job->fd = -1;
-	job->event = NULL;
-
-	job->callbackfn = callbackfn;
-	job->freefn = freefn;
-	job->data = data;
-
-	job->flags = flags;
-
-	if (jobs != NULL)
-		RB_INSERT(jobs, jobs, job);
-	LIST_INSERT_HEAD(&all_jobs, job, lentry);
-
-	return (job);
-}
-
-/* Remove job from tree and free. */
-void
-job_remove(struct jobs *jobs, struct job *job)
-{
-	if (jobs != NULL)
-		RB_REMOVE(jobs, jobs, job);
-	job_free(job);
-}
-
-/* Kill and free an individual job. */
-void
-job_free(struct job *job)
-{
-	job_kill(job);
-
-	LIST_REMOVE(job, lentry);
-	xfree(job->cmd);
-
-	if (job->freefn != NULL && job->data != NULL)
-		job->freefn(job->data);
-
-	if (job->fd != -1)
-		close(job->fd);
-	if (job->event != NULL)
-		bufferevent_free(job->event);
-
-	xfree(job);
-}
-
-/* Start a job running, if it isn't already. */
-int
-job_run(struct job *job)
-{
-	struct environ	env;
-	int		nullfd, out[2];
-
-	if (job->fd != -1 || job->pid != -1)
-		return (0);
 
 	if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, out) != 0)
-		return (-1);
+		return (NULL);
 
 	environ_init(&env);
 	environ_copy(&global_environ, &env);
 	server_fill_environ(NULL, &env);
 
-	switch (job->pid = fork()) {
+	switch (pid = fork()) {
 	case -1:
 		environ_free(&env);
-		return (-1);
+		return (NULL);
 	case 0:		/* child */
 		clear_signals(1);
 
@@ -178,23 +81,55 @@ job_run(struct job *job)
 
 		closefrom(STDERR_FILENO + 1);
 
-		execl(_PATH_BSHELL, "sh", "-c", job->cmd, (char *) NULL);
+		execl(_PATH_BSHELL, "sh", "-c", cmd, (char *) NULL);
 		fatal("execl failed");
-	default:	/* parent */
-		environ_free(&env);
-		close(out[1]);
-
-		job->fd = out[0];
-		setblocking(job->fd, 0);
-
-		if (job->event != NULL)
-			bufferevent_free(job->event);
-		job->event =
-		    bufferevent_new(job->fd, NULL, NULL, job_callback, job);
-		bufferevent_enable(job->event, EV_READ);
-
-		return (0);
 	}
+
+	/* parent */
+	environ_free(&env);
+	close(out[1]);
+
+	job = xmalloc(sizeof *job);
+	job->cmd = xstrdup(cmd);
+	job->pid = pid;
+	job->status = 0;
+
+	LIST_INSERT_HEAD(&all_jobs, job, lentry);
+
+	job->callbackfn = callbackfn;
+	job->freefn = freefn;
+	job->data = data;
+
+	job->fd = out[0];
+	setblocking(job->fd, 0);
+
+	job->event = bufferevent_new(job->fd, NULL, NULL, job_callback, job);
+	bufferevent_enable(job->event, EV_READ);
+
+	log_debug("run job %p: %s, pid %ld", job, job->cmd, (long) job->pid);
+	return (job);
+}
+
+/* Kill and free an individual job. */
+void
+job_free(struct job *job)
+{
+	log_debug("free job %p: %s", job, job->cmd);
+
+	LIST_REMOVE(job, lentry);
+	xfree(job->cmd);
+
+	if (job->freefn != NULL && job->data != NULL)
+		job->freefn(job->data);
+
+	if (job->pid != -1)
+		kill(job->pid, SIGTERM);
+	if (job->fd != -1)
+		close(job->fd);
+	if (job->event != NULL)
+		bufferevent_free(job->event);
+
+	xfree(job);
 }
 
 /* Job buffer error callback. */
@@ -204,15 +139,16 @@ job_callback(unused struct bufferevent *bufev, unused short events, void *data)
 {
 	struct job	*job = data;
 
-	bufferevent_disable(job->event, EV_READ);
+	log_debug("job error %p: %s, pid %ld", job, job->cmd, (long) job->pid);
-	close(job->fd);
-	job->fd = -1;
 
 	if (job->pid == -1) {
 		if (job->callbackfn != NULL)
 			job->callbackfn(job);
-		if ((!job->flags & JOB_PERSIST))
+		job_free(job);
-			job_free(job);
+	} else {
+		bufferevent_disable(job->event, EV_READ);
+		close(job->fd);
+		job->fd = -1;
 	}
 }
 
@@ -220,23 +156,14 @@ job_callback(unused struct bufferevent *bufev, unused short events, void *data)
 void
 job_died(struct job *job, int status)
 {
+	log_debug("job died %p: %s, pid %ld", job, job->cmd, (long) job->pid);
+
 	job->status = status;
-	job->pid = -1;
 
 	if (job->fd == -1) {
 		if (job->callbackfn != NULL)
 			job->callbackfn(job);
-		if ((!job->flags & JOB_PERSIST))
+		job_free(job);
-			job_free(job);
+	} else
-	}
+		job->pid = -1;
-}
-
-/* Kill a job. */
-void
-job_kill(struct job *job)
-{
-	if (job->pid == -1)
-		return;
-	kill(job->pid, SIGTERM);
-	job->pid = -1;
 }

server-client.c

@@ -79,7 +79,8 @@ server_client_create(int fd)
 	c->tty.sy = 24;
 
 	screen_init(&c->status, c->tty.sx, 1, 0);
-	job_tree_init(&c->status_jobs);
+	RB_INIT(&c->status_new);
+	RB_INIT(&c->status_old);
 
 	c->message_string = NULL;
 	ARRAY_INIT(&c->message_log);
@@ -139,8 +140,9 @@ server_client_lost(struct client *c)
 	if (c->stderr_event != NULL)
 		bufferevent_free(c->stderr_event);
 
+	status_free_jobs(&c->status_new);
+	status_free_jobs(&c->status_old);
 	screen_free(&c->status);
-	job_tree_free(&c->status_jobs);
 
 	if (c->title != NULL)
 		xfree(c->title);
@@ -221,7 +223,6 @@ server_client_status_timer(void)
 {
 	struct client	*c;
 	struct session	*s;
-	struct job	*job;
 	struct timeval	 tv;
 	u_int		 i;
 	int		 interval;
@@ -250,8 +251,7 @@ server_client_status_timer(void)
 
 		difference = tv.tv_sec - c->status_timer.tv_sec;
 		if (difference >= interval) {
-			RB_FOREACH(job, jobs, &c->status_jobs)
+			status_update_jobs(c);
-				job_run(job);
 			c->flags |= CLIENT_STATUS;
 		}
 	}

status.c

@@ -33,7 +33,8 @@ char   *status_redraw_get_left(
 	    struct client *, time_t, int, struct grid_cell *, size_t *);
 char   *status_redraw_get_right(
 	    struct client *, time_t, int, struct grid_cell *, size_t *);
-char   *status_job(struct client *, char **);
+char   *status_find_job(struct client *, char **);
+void	status_job_free(void *);
 void	status_job_callback(struct job *);
 char   *status_print(
 	    struct client *, struct winlink *, time_t, struct grid_cell *);
@@ -49,6 +50,16 @@ char   *status_prompt_complete(const char *);
 /* Status prompt history. */
 ARRAY_DECL(, char *) status_prompt_history = ARRAY_INITIALIZER;
 
+/* Status output tree. */
+RB_GENERATE(status_out_tree, status_out, entry, status_out_cmp);
+
+/* Output tree comparison function. */
+int
+status_out_cmp(struct status_out *so1, struct status_out *so2)
+{
+	return (strcmp(so1->cmd, so2->cmd));
+}
+
 /* Retrieve options for left string. */
 char *
 status_redraw_get_left(struct client *c,
@@ -365,9 +376,8 @@ status_replace1(struct client *c,struct winlink *wl,
 			ch = ')';
 			goto skip_to;
 		}
-		if ((ptr = status_job(c, iptr)) == NULL)
+		if ((ptr = status_find_job(c, iptr)) == NULL)
 			return;
-		freeptr = ptr;
 		goto do_replace;
 	case 'H':
 		if (gethostname(tmp, sizeof tmp) != 0)
@@ -469,12 +479,12 @@ status_replace(struct client *c,
 
 /* Figure out job name and get its result, starting it off if necessary. */
 char *
-status_job(struct client *c, char **iptr)
+status_find_job(struct client *c, char **iptr)
 {
-	struct job	*job;
+	struct status_out	*so, so_find;
-	char   		*cmd;
+	char   			*cmd;
-	int		 lastesc;
+	int			 lastesc;
-	size_t		 len;
+	size_t			 len;
 
 	if (**iptr == '\0')
 		return (NULL);
@@ -504,24 +514,88 @@ status_job(struct client *c, char **iptr)
 	(*iptr)++;			/* skip final ) */
 	cmd[len] = '\0';
 
-	job = job_get(&c->status_jobs, cmd);
+	/* First try in the new tree. */
-	if (job == NULL) {
+	so_find.cmd = cmd;
-		job = job_add(&c->status_jobs,
+	so = RB_FIND(status_out_tree, &c->status_new, &so_find);
-		    JOB_PERSIST, c, cmd, status_job_callback, xfree, NULL);
+	if (so != NULL && so->out != NULL)
-		job_run(job);
+		return (so->out);
+
+	/* If not found at all, start the job and add to the tree. */
+	if (so == NULL) {
+		job_run(cmd, status_job_callback, status_job_free, c);
+		c->references++;
+
+		so = xmalloc(sizeof *so);
+		so->cmd = xstrdup(cmd);
+		so->out = NULL;
+		RB_INSERT(status_out_tree, &c->status_new, so);
 	}
+
+	/* Lookup in the old tree. */
+	so_find.cmd = cmd;
+	so = RB_FIND(status_out_tree, &c->status_old, &so_find);
 	xfree(cmd);
-	if (job->data == NULL)
+	if (so != NULL)
-		return (xstrdup(""));
+		return (so->out);
-	return (xstrdup(job->data));
+	return (NULL);
+}
+
+/* Free job tree. */
+void
+status_free_jobs(struct status_out_tree *sotree)
+{
+	struct status_out	*so, *so_next;
+
+	so_next = RB_MIN(status_out_tree, sotree);
+	while (so_next != NULL) {
+		so = so_next;
+		so_next = RB_NEXT(status_out_tree, sotree, so);
+
+		RB_REMOVE(status_out_tree, sotree, so);
+		if (so->out != NULL)
+			xfree(so->out);
+		xfree(so->cmd);
+		xfree(so);
+	}
+}
+
+/* Update jobs on status interval. */
+void
+status_update_jobs(struct client *c)
+{
+	/* Free the old tree. */
+	status_free_jobs(&c->status_old);
+
+	/* Move the new to old. */
+	memcpy(&c->status_old, &c->status_new, sizeof c->status_old);
+	RB_INIT(&c->status_new);
+}
+
+/* Free status job. */
+void
+status_job_free(void *data)
+{
+	struct client	*c = data;
+
+	c->references--;
 }
 
 /* Job has finished: save its result. */
 void
 status_job_callback(struct job *job)
 {
-	char	*line, *buf;
+	struct client		*c = job->data;
-	size_t	 len;
+	struct status_out	*so, so_find;
+	char			*line, *buf;
+	size_t			 len;
+
+	if (c->flags & CLIENT_DEAD)
+		return;
+
+	so_find.cmd = job->cmd;
+	so = RB_FIND(status_out_tree, &c->status_new, &so_find);
+	if (so == NULL || so->out != NULL)
+		return;
 
 	buf = NULL;
 	if ((line = evbuffer_readline(job->event->input)) == NULL) {
@@ -530,17 +604,11 @@ status_job_callback(struct job *job)
 		if (len != 0)
 			memcpy(buf, EVBUFFER_DATA(job->event->input), len);
 		buf[len] = '\0';
-	}
+	} else
+		buf = xstrdup(line);
 
-	if (job->data != NULL)
+	so->out = buf;
-		xfree(job->data);
+	server_redraw_client(c);
-	else
-		server_redraw_client(job->client);
-
-	if (line == NULL)
-		job->data = buf;
-	else
-		job->data = xstrdup(line);
 }
 
 /* Return winlink status line entry and adjust gc as necessary. */

tmux.h

@@ -671,8 +671,6 @@ struct job {
 	pid_t		 pid;
 	int		 status;
 
-	struct client	*client;
-
 	int		 fd;
 	struct bufferevent *event;
 
@@ -680,13 +678,8 @@ struct job {
 	void		(*freefn)(void *);
 	void		*data;
 
-	int		 flags;
-#define JOB_PERSIST 0x1	/* don't free after callback */
-
-	RB_ENTRY(job)	 entry;
 	LIST_ENTRY(job)	 lentry;
 };
-RB_HEAD(jobs, job);
 LIST_HEAD(joblist, job);
 
 /* Screen selection. */
@@ -1091,6 +1084,15 @@ struct message_entry {
 	time_t	msg_time;
 };
 
+/* Status output data from a job. */
+struct status_out {
+	char   *cmd;
+	char   *out;
+
+	RB_ENTRY(status_out) entry;
+};
+RB_HEAD(status_out_tree, status_out);
+
 /* Client connection. */
 struct client {
 	struct imsgbuf	 ibuf;
@@ -1120,8 +1122,9 @@ struct client {
 
 	struct event	 repeat_timer;
 
+	struct status_out_tree status_old;
+	struct status_out_tree status_new;
 	struct timeval	 status_timer;
-	struct jobs	 status_jobs;
 	struct screen	 status;
 
 #define CLIENT_TERMINAL 0x1
@@ -1363,18 +1366,10 @@ const char *options_table_print_entry(
 
 /* job.c */
 extern struct joblist all_jobs;
-int	job_cmp(struct job *, struct job *);
+struct job *job_run(
-RB_PROTOTYPE(jobs, job, entry, job_cmp);
-void	job_tree_init(struct jobs *);
-void	job_tree_free(struct jobs *);
-struct job *job_get(struct jobs *, const char *);
-struct job *job_add(struct jobs *, int, struct client *,
 	    const char *, void (*)(struct job *), void (*)(void *), void *);
-void	job_remove(struct jobs *, struct job *);
 void	job_free(struct job *);
-int	job_run(struct job *);
 void	job_died(struct job *, int);
-void	job_kill(struct job *);
 
 /* environ.c */
 int	environ_cmp(struct environ_entry *, struct environ_entry *);
@@ -1660,6 +1655,10 @@ void	 server_clear_identify(struct client *);
 void	 server_update_event(struct client *);
 
 /* status.c */
+int	 status_out_cmp(struct status_out *, struct status_out *);
+RB_PROTOTYPE(status_out_tree, status_out, entry, status_out_cmp);
+void	 status_free_jobs(struct status_out_tree *);
+void	 status_update_jobs(struct client *);
 int	 status_redraw(struct client *);
 char	*status_replace(
 	     struct client *, struct winlink *, const char *, time_t, int);