Rather than running status-left, status-right and window title #() with popen

immediately every redraw, queue them up and run them in the background,
starting each once every status-interval. The actual status line uses the
output from the last run.

This brings several advantages:

- tmux itself may be called from inside #() without causing the server to hang;
- likewise, sleep or similar doesn't cause the server to block;
- commands aren't run excessively often when redrawing;
- commands shared by status-left and status-right, or used multiple times, will
  only be run once.

run-shell and if-shell still use system()/popen() but will be changed over to
use this too later.

Makefile

@@ -30,7 +30,7 @@ SRCS=	attributes.c buffer-poll.c buffer.c cfg.c client-fn.c \
 	cmd-up-pane.c cmd-display-message.c cmd-display-panes.c cmd.c \
 	colour.c environ.c grid-view.c grid.c input-keys.c \
 	imsg.c imsg-buffer.c input.c key-bindings.c key-string.c \
-	layout-set.c layout.c log.c \
+	layout-set.c layout.c log.c job.c \
 	mode-key.c names.c options-cmd.c options.c paste.c procname.c \
 	resize.c screen-redraw.c screen-write.c screen.c server-fn.c \
 	server-msg.c server.c session.c status.c tmux.c tty-keys.c tty-term.c \

buffer-poll.c

@@ -29,9 +29,9 @@ buffer_poll(struct pollfd *pfd, struct buffer *in, struct buffer *out)
 {
 	ssize_t	n;
 
-	if (pfd->revents & (POLLERR|POLLNVAL|POLLHUP))
+	if (pfd->revents & (POLLERR|POLLNVAL))
 		return (-1);
-	if (pfd->revents & POLLIN) {
+	if (in != NULL && pfd->revents & POLLIN) {
 		buffer_ensure(in, BUFSIZ);
 		n = read(pfd->fd, BUFFER_IN(in), BUFFER_FREE(in));
 		if (n == 0)
@@ -41,8 +41,9 @@ buffer_poll(struct pollfd *pfd, struct buffer *in, struct buffer *out)
 				return (-1);
 		} else
 			buffer_add(in, n);
-	}
-	if (BUFFER_USED(out) > 0 && pfd->revents & POLLOUT) {
+	} else if (pfd->revents & POLLHUP)
+		return (-1);
+	if (out != NULL && BUFFER_USED(out) > 0 && pfd->revents & POLLOUT) {
 		n = write(pfd->fd, BUFFER_OUT(out), BUFFER_USED(out));
 		if (n == -1) {
 			if (errno != EINTR && errno != EAGAIN)

cmd-display-message.c

@@ -55,7 +55,7 @@ cmd_display_message_exec(struct cmd *self, struct cmd_ctx *ctx)
 	else
 		template = data->arg;
 
-	msg = status_replace(c->session, template, time(NULL));
+	msg = status_replace(c, template, time(NULL));
 	status_message_set(c, "%s", msg);
 	xfree(msg);
 

cmd-set-option.c

@@ -186,9 +186,12 @@ cmd_set_option_exec(struct cmd *self, struct cmd_ctx *ctx)
 	recalculate_sizes();
 	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) {
+			job_tree_free(&c->status_jobs);
+			job_tree_init(&c->status_jobs);
 			server_redraw_client(c);
 		}
+	}
 
 	return (0);
 }

job.c

@@ -0,0 +1,187 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2009 Nicholas Marriott <nicm@users.sourceforge.net>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF MIND, USE, DATA OR PROFITS, WHETHER
+ * IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING
+ * OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <sys/types.h>
+
+#include <fcntl.h>
+#include <paths.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "tmux.h"
+
+/*
+ * Job scheduling. Run queued commands in the background and record their
+ * output.
+ */
+
+RB_GENERATE(jobs, job, entry, job_cmp);
+
+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);
+}
+
+/* Count the number of jobs in a tree. */
+u_int
+job_tree_size(struct jobs *jobs)
+{
+	struct job	*job;
+	u_int		 n;
+
+	n = 0;
+	RB_FOREACH(job, jobs, jobs)
+		n++;
+	return (n);
+}
+
+/* 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)
+{
+	struct job	job;
+
+	job.cmd = (char *) cmd;
+	return (RB_FIND(jobs, jobs, &job));
+}
+
+/* Add a job. */
+struct job *
+job_add(struct jobs *jobs, struct client *c, const char *cmd,
+    void (*callbackfn)(struct job *), void (*freefn)(void *), void *data)
+{
+	struct job	*job;
+ 
+	job = xmalloc(sizeof *job);
+	job->cmd = xstrdup(cmd);
+
+	job->client = c;
+
+	job->fd = -1;
+	job->out = buffer_create(BUFSIZ);
+
+	job->callbackfn = callbackfn;
+	job->freefn = freefn;
+	job->data = data;
+
+	RB_INSERT(jobs, jobs, job);
+	
+	return (job);
+}
+
+/* Kill and free an individual job. */
+void
+job_free(struct job *job)
+{
+	job_kill(job);
+
+	xfree(job->cmd);
+
+	if (job->fd != -1)
+		close(job->fd);
+	if (job->out != NULL)
+		buffer_destroy(job->out);
+
+	xfree(job);
+}
+
+/* Start a job running, if it isn't already. */
+int
+job_run(struct job *job)
+{
+	int	nullfd, out[2], mode;
+
+	if (job->fd != -1)
+		return (0);
+
+	if (pipe(out) != 0)
+		return (-1);
+
+	switch (job->pid = fork()) {
+	case -1:
+		return (-1);
+	case 0:		/* child */
+		sigreset();
+		/* XXX environ? */
+
+		nullfd = open(_PATH_DEVNULL, O_RDONLY, 0);
+		if (nullfd < 0)
+			fatal("open failed");
+		if (dup2(nullfd, STDIN_FILENO) == -1)
+			fatal("dup2 failed");
+		if (dup2(nullfd, STDERR_FILENO) == -1)
+			fatal("dup2 failed");
+		if (nullfd != STDIN_FILENO && nullfd != STDERR_FILENO)
+			close(nullfd);
+
+		close(out[1]);
+		if (dup2(out[0], STDOUT_FILENO) == -1)
+			fatal("dup2 failed");
+		if (out[0] != STDOUT_FILENO)
+			close(out[0]);
+
+		execl(_PATH_BSHELL, "sh", "-c", job->cmd, (char *) NULL);
+		fatal("execl failed");
+	default:	/* parent */
+		close(out[0]);
+
+		job->fd = out[1];
+		if ((mode = fcntl(job->fd, F_GETFL)) == -1)
+			fatal("fcntl failed");
+		if (fcntl(job->fd, F_SETFL, mode|O_NONBLOCK) == -1)
+			fatal("fcntl failed");
+		if (fcntl(job->fd, F_SETFD, FD_CLOEXEC) == -1)
+			fatal("fcntl failed");
+
+		if (BUFFER_USED(job->out) != 0)
+			buffer_remove(job->out, BUFFER_USED(job->out));
+
+		return (0);
+	}
+}
+
+/* Kill a job. */
+void
+job_kill(struct job *job)
+{
+	if (job->pid == -1)
+		return;
+	kill(job->pid, SIGTERM);
+	job->pid = -1;
+}

server.c

@@ -72,6 +72,10 @@ void		 server_fill_windows(void);
 void		 server_handle_windows(void);
 void		 server_fill_clients(void);
 void		 server_handle_clients(void);
+void		 server_fill_jobs(void);
+void		 server_fill_jobs1(struct jobs *);
+void		 server_handle_jobs(void);
+void		 server_handle_jobs1(struct jobs *);
 void		 server_accept_client(int);
 void		 server_handle_client(struct client *);
 void		 server_handle_window(struct window *, struct window_pane *);
@@ -190,7 +194,9 @@ server_create_client(int fd)
 	c->session = NULL;
 	c->tty.sx = 80;
 	c->tty.sy = 24;
+
 	screen_init(&c->status, c->tty.sx, 1, 0);
+	job_tree_init(&c->status_jobs);
 
 	c->message_string = NULL;
 
@@ -370,6 +376,7 @@ server_main(int srv_fd)
 		server_poll_add(srv_fd, POLLIN);
 
 		/* Fill window and client sockets. */
+		server_fill_jobs();
 		server_fill_windows();
 		server_fill_clients();
 
@@ -412,6 +419,7 @@ server_main(int srv_fd)
 		 * windows, so windows must come first to avoid messing up by
 		 * increasing the array size.
 		 */
+		server_handle_jobs();
 		server_handle_windows();
 		server_handle_clients();
 
@@ -648,7 +656,7 @@ server_set_title(struct client *c)
 
 	template = options_get_string(&s->options, "set-titles-string");
 	
-	title = status_replace(c->session, template, time(NULL));
+	title = status_replace(c, template, time(NULL));
 	if (c->title == NULL || strcmp(title, c->title) != 0) {
 		if (c->title != NULL)
 			xfree(c->title);
@@ -663,6 +671,7 @@ void
 server_check_timers(struct client *c)
 {
 	struct session	*s;
+	struct job	*job;
 	struct timeval	 tv;
 	u_int		 interval;
 
@@ -694,8 +703,12 @@ server_check_timers(struct client *c)
 	if (interval == 0)
 		return;
 	if (tv.tv_sec < c->status_timer.tv_sec ||
-	    ((u_int) tv.tv_sec) - c->status_timer.tv_sec >= interval)
+	    ((u_int) tv.tv_sec) - c->status_timer.tv_sec >= interval) {
+		/* Run the jobs for this client and schedule for redraw. */
+		RB_FOREACH(job, jobs, &c->status_jobs)
+			job_run(job);
 		c->flags |= CLIENT_STATUS;
+	}
 }
 
 /* Fill client pollfds. */
@@ -747,6 +760,66 @@ server_fill_clients(void)
 	}
 }
 
+/* Fill in job fds. */
+void
+server_fill_jobs(void)
+{
+ 	struct client	*c;
+	u_int		 i;
+
+	for (i = 0; i < ARRAY_LENGTH(&clients); i++) {
+		c = ARRAY_ITEM(&clients, i);
+		if (c != NULL)
+			server_fill_jobs1(&c->status_jobs);
+	}
+}
+
+void
+server_fill_jobs1(struct jobs *jobs)
+{
+	struct job	*job;
+
+	RB_FOREACH(job, jobs, jobs) {
+		if (job->fd == -1)
+			continue;
+		server_poll_add(job->fd, POLLIN);
+	}
+}
+
+/* Handle job fds. */
+void
+server_handle_jobs(void)
+{
+	struct client	*c;
+	u_int		 i;
+	
+	for (i = 0; i < ARRAY_LENGTH(&clients); i++) {
+		c = ARRAY_ITEM(&clients, i);
+		if (c != NULL)
+			server_handle_jobs1(&c->status_jobs);
+	}
+}
+
+void
+server_handle_jobs1(struct jobs *jobs)
+{
+	struct job	*job;
+	struct pollfd	*pfd;
+
+	RB_FOREACH(job, jobs, jobs) {
+		if (job->fd == -1)
+			continue;
+		if ((pfd = server_poll_lookup(job->fd)) == NULL)
+			continue;
+		if (buffer_poll(pfd, job->out, NULL) != 0) {
+			close(job->fd);
+			job->fd = -1;
+			if (job->callbackfn != NULL)
+				job->callbackfn(job);
+		}
+	}
+}
+
 /* Handle client pollfds. */
 void
 server_handle_clients(void)
@@ -991,6 +1064,7 @@ server_lost_client(struct client *c)
 		tty_free(&c->tty);
 
 	screen_free(&c->status);
+	job_tree_free(&c->status_jobs);
 
 	if (c->title != NULL)
 		xfree(c->title);

status.c

@@ -29,7 +29,8 @@
 
 #include "tmux.h"
 
-char   *status_replace_popen(char **);
+char   *status_job(struct client *, char **);
+void	status_job_callback(struct job *);
 size_t	status_width(struct winlink *);
 char   *status_print(struct session *, struct winlink *, struct grid_cell *);
 
@@ -104,14 +105,14 @@ status_redraw(struct client *c)
 	utf8flag = options_get_number(&s->options, "status-utf8");
 
 	/* Work out the left and right strings. */
-	left = status_replace(s, options_get_string(
+	left = status_replace(c, options_get_string(
 	    &s->options, "status-left"), c->status_timer.tv_sec);
 	llen = options_get_number(&s->options, "status-left-length");
 	llen2 = screen_write_cstrlen(utf8flag, "%s", left);
 	if (llen2 < llen)
 		llen = llen2;
 
-	right = status_replace(s, options_get_string(
+	right = status_replace(c, options_get_string(
 	    &s->options, "status-right"), c->status_timer.tv_sec);
 	rlen = options_get_number(&s->options, "status-right-length");
 	rlen2 = screen_write_cstrlen(utf8flag, "%s", right);
@@ -317,15 +318,17 @@ out:
 }
 
 char *
-status_replace(struct session *s, const char *fmt, time_t t)
+status_replace(struct client *c, const char *fmt, time_t t)
 {
+	struct session *s = c->session;
 	struct winlink *wl = s->curw;
 	static char	out[BUFSIZ];
 	char		in[BUFSIZ], tmp[256], ch, *iptr, *optr, *ptr, *endptr;
-	char           *savedptr;
+	char           *savedptr;	/* freed at end of each loop */
 	size_t		len;
 	long		n;
 	
+	
 	strftime(in, sizeof in, fmt, localtime(&t));
 	in[(sizeof in) - 1] = '\0';
 
@@ -352,7 +355,7 @@ status_replace(struct session *s, const char *fmt, time_t t)
 			switch (*iptr++) {
 			case '(':
 				if (ptr == NULL) {
-					ptr = status_replace_popen(&iptr);
+					ptr = status_job(c, &iptr);
 					if (ptr == NULL)
 						break;
 					savedptr = ptr;
@@ -434,10 +437,10 @@ status_replace(struct session *s, const char *fmt, time_t t)
 }
 
 char *
-status_replace_popen(char **iptr)
+status_job(struct client *c, char **iptr)
 {
-	FILE	*f;
-	char	*buf, *cmd, *ptr;
+	struct job	*job;
+	char   		*buf, *cmd;
 	int		 lastesc;
 	size_t		 len;
 
@@ -464,32 +467,44 @@ status_replace_popen(char **iptr)
 		lastesc = 0;
 		cmd[len++] = **iptr;
 	}
-	if (**iptr == '\0')		/* no terminating ) */
-		goto out;
+	if (**iptr == '\0')		/* no terminating ) */ {
+		xfree(cmd);
+		return (NULL);
+	}
 	(*iptr)++;			/* skip final ) */
 	cmd[len] = '\0';
 
-	if ((f = popen(cmd, "r")) == NULL)
-		goto out;
-
-	if ((buf = fgetln(f, &len)) == NULL) {
-		pclose(f);
-		goto out;
+	job = job_get(&c->status_jobs, cmd);
+	if (job == NULL) {
+		job = job_add(
+		    &c->status_jobs, c, cmd, status_job_callback, xfree, NULL);
+		job_run(job);
 	}
-	if (buf[len - 1] == '\n') {
-		buf[len - 1] = '\0';
-		buf = xstrdup(buf);
-	} else {
-		ptr = xmalloc(len + 1);
-		memcpy(ptr, buf, len);
-		ptr[len] = '\0';
-		buf = ptr;
-	}
-	pclose(f);
+	if (job->data == NULL)
+		return (xstrdup(""));
+	return (xstrdup(job->data));
+}
 
-out:
-	xfree(cmd);
-	return (buf);
+void
+status_job_callback(struct job *job)
+{
+	char	*buf;
+	size_t	 len;
+
+	len = BUFFER_USED(job->out);
+	buf = xmalloc(len + 1);
+	if (len != 0)
+		buffer_read(job->out, buf, len);
+	buf[len] = '\0';
+	buf[strcspn(buf, "\n")] = '\0';
+
+	if (job->data != NULL)
+		xfree(job->data);
+	else
+		server_redraw_client(job->client);
+	job->data = xstrdup(buf);
+
+	xfree(buf);
 }
 
 size_t

tmux.1

@@ -1437,6 +1437,11 @@ may contain any of the following special character sequences:
 The #(command) form executes
 .Ql command
 as a shell command and inserts the first line of its output.
+Note that shell commands are only executed once at the interval specified by
+the
+.Ic status-interval
+option: if the status line is redrawn in the meantime, the previous result is
+used.
 #[attributes] allows a comma-separated list of attributes to be specified,
 these may be
 .Ql fg=colour

tmux.h

@@ -563,6 +563,24 @@ struct options {
 /* Key list for prefix option. */
 ARRAY_DECL(keylist, int);
 
+/* Scheduled job. */
+struct job {
+	char		*cmd;
+	pid_t		 pid;
+
+	struct client	*client;
+
+	int		 fd;
+	struct buffer	*out;
+
+	void		(*callbackfn)(struct job *);
+	void		(*freefn)(void *);
+	void		*data;
+
+	RB_ENTRY(job)	 entry;
+};
+RB_HEAD(jobs, job);
+
 /* Screen selection. */
 struct screen_sel {
 	int		 flag;
@@ -942,9 +960,10 @@ struct client {
 	char		*cwd;
 
 	struct tty 	 tty;
-	struct timeval	 status_timer;
 	struct timeval	 repeat_timer;
 
+	struct timeval	 status_timer;
+	struct jobs	 status_jobs;
 	struct screen	 status;
 
 #define CLIENT_TERMINAL 0x1
@@ -1179,6 +1198,20 @@ struct options_entry *options_set_data(
     	    struct options *, const char *, void *, void (*)(void *));
 void   *options_get_data(struct options *, const char *);
 
+/* job.c */
+extern struct jobs jobs_tree;
+int	job_cmp(struct job *, struct job *);
+RB_PROTOTYPE(jobs, job, entry, job_cmp);
+void	job_tree_init(struct jobs *);
+void	job_tree_free(struct jobs *);
+u_int	job_tree_size(struct jobs *);
+struct job *job_get(struct jobs *, const char *);
+struct job *job_add(struct jobs *, struct client *,
+	    const char *, void (*)(struct job *), void (*)(void *), void *);
+void	job_free(struct job *);
+int	job_run(struct job *);
+void	job_kill(struct job *);
+
 /* environ.c */
 int	environ_cmp(struct environ_entry *, struct environ_entry *);
 RB_PROTOTYPE(environ, environ_entry, entry, environ_cmp);
@@ -1485,7 +1518,7 @@ void	 server_clear_identify(struct client *);
 
 /* status.c */
 int	 status_redraw(struct client *);
-char	*status_replace(struct session *, const char *, time_t);
+char	*status_replace(struct client *, const char *, time_t);
 void printflike2 status_message_set(struct client *, const char *, ...);
 void	 status_message_clear(struct client *);
 int	 status_message_redraw(struct client *);