Sync OpenBSD patchset 741:

When changing so that the client passes its stdout and stderr as well as
stdin up to the server, I forgot one essential point - the tmux server
could now be both the producer and consumer. This happens when tmux is
run inside tmux, as well as when piping tmux commands together.

So, using stdio(3) was a bad idea - if sufficient data was written, this
could block in write(2). When that happened and the server was both
producer and consumer, it deadlocks.

Change to use libevent bufferevents for the client stdin, stdout and
stderr instead. This is trivial enough for output but requires a
callback mechanism to trigger when stdin is finished.

This relies on the underlying polling mechanism for libevent to work
with whatever devices to which the user could redirect stdin, stdout or
stderr, hence the change to use poll(2) over kqueue(2) for tmux.

Author: tcunha

cmd-if-shell.c

@@ -1,4 +1,4 @@
-/* $Id: cmd-if-shell.c,v 1.9 2010-07-17 14:36:40 tcunha Exp $ */
+/* $Id: cmd-if-shell.c,v 1.10 2010-08-09 21:44:25 tcunha Exp $ */
 
 /*
  * Copyright (c) 2009 Tiago Cunha <me@tiagocunha.org>
@@ -109,8 +109,7 @@ cmd_if_shell_free(void *data)
 	if (ctx->cmdclient != NULL) {
 		ctx->cmdclient->references--;
 		exitdata.retcode = ctx->cmdclient->retcode;
-		server_write_client(
-		    ctx->cmdclient, MSG_EXIT, &exitdata, sizeof exitdata);
+		ctx->cmdclient->flags |= CLIENT_EXIT;
 	}
 	if (ctx->curclient != NULL)
 		ctx->curclient->references--;

cmd-load-buffer.c

@@ -1,4 +1,4 @@
-/* $Id: cmd-load-buffer.c,v 1.16 2010-07-02 02:52:13 tcunha Exp $ */
+/* $Id: cmd-load-buffer.c,v 1.17 2010-08-09 21:44:25 tcunha Exp $ */
 
 /*
  * Copyright (c) 2009 Tiago Cunha <me@tiagocunha.org>
@@ -31,6 +31,7 @@
  */
 
 int	cmd_load_buffer_exec(struct cmd *, struct cmd_ctx *);
+void	cmd_load_buffer_callback(struct client *, void *);
 
 const struct cmd_entry cmd_load_buffer_entry = {
 	"load-buffer", "loadb",
@@ -43,37 +44,55 @@ const struct cmd_entry cmd_load_buffer_entry = {
 	cmd_buffer_print
 };
 
+struct cmd_load_buffer_cdata {
+	struct session	*session;
+	int		 buffer;
+};
+
 int
 cmd_load_buffer_exec(struct cmd *self, struct cmd_ctx *ctx)
 {
-	struct cmd_buffer_data	*data = self->data;
-	struct session		*s;
-	FILE			*f, *close_f;
-	char		      	*pdata, *new_pdata;
-	size_t			 psize;
-	u_int			 limit;
-	int			 ch;
+	struct cmd_buffer_data		*data = self->data;
+	struct cmd_load_buffer_cdata	*cdata;
+	struct session			*s;
+	struct client			*c = ctx->cmdclient;
+	FILE				*f;
+	char		      		*pdata, *new_pdata;
+	size_t				 psize;
+	u_int				 limit;
+	int				 ch;
 
 	if ((s = cmd_find_session(ctx, data->target)) == NULL)
 		return (-1);
 
-	if (strcmp(data->arg, "-") == 0 ) {
-		if (ctx->cmdclient == NULL) {
+	if (strcmp(data->arg, "-") == 0) {
+		if (c == NULL) {
 			ctx->error(ctx, "%s: can't read from stdin", data->arg);
 			return (-1);
 		}
-		f = ctx->cmdclient->stdin_file;
-		if (isatty(fileno(ctx->cmdclient->stdin_file))) {
+		if (c->flags & CLIENT_TERMINAL) {
 			ctx->error(ctx, "%s: stdin is a tty", data->arg);
 			return (-1);
 		}
-		close_f = NULL;
-	} else {
-		if ((f = fopen(data->arg, "rb")) == NULL) {
-			ctx->error(ctx, "%s: %s", data->arg, strerror(errno));
+		if (c->stdin_fd == -1) {
+			ctx->error(ctx, "%s: can't read from stdin", data->arg);
 			return (-1);
 		}
-		close_f = f;
+
+		cdata = xmalloc(sizeof *cdata);
+		cdata->session = s;
+		cdata->buffer = data->buffer;
+		c->stdin_data = cdata;
+		c->stdin_callback = cmd_load_buffer_callback;
+
+		c->references++;
+		bufferevent_enable(c->stdin_event, EV_READ);
+		return (1);
+	}
+
+	if ((f = fopen(data->arg, "rb")) == NULL) {
+		ctx->error(ctx, "%s: %s", data->arg, strerror(errno));
+		return (-1);
 	}
 
 	pdata = NULL;
@@ -94,8 +113,8 @@ cmd_load_buffer_exec(struct cmd *self, struct cmd_ctx *ctx)
 	if (pdata != NULL)
 		pdata[psize] = '\0';
 
-	if (close_f != NULL)
-		fclose(close_f);
+	fclose(f);
+	f = NULL;
 
 	limit = options_get_number(&s->options, "buffer-limit");
 	if (data->buffer == -1) {
@@ -104,15 +123,62 @@ cmd_load_buffer_exec(struct cmd *self, struct cmd_ctx *ctx)
 	}
 	if (paste_replace(&s->buffers, data->buffer, pdata, psize) != 0) {
 		ctx->error(ctx, "no buffer %d", data->buffer);
-		goto error;
+		return (-1);
 	}
 
 	return (0);
 
 error:
 	if (pdata != NULL)
 		xfree(pdata);
-	if (close_f != NULL)
-		fclose(close_f);
+	if (f != NULL)
+		fclose(f);
 	return (-1);
 }
+
+void
+cmd_load_buffer_callback(struct client *c, void *data)
+{
+	struct cmd_load_buffer_cdata	*cdata = data;
+	struct session			*s = cdata->session;
+	char				*pdata;
+	size_t				 psize;
+	u_int				 limit;
+	int				 idx;
+
+	/*
+	 * Event callback has already checked client is not dead and reduced
+	 * its reference count. But tell it to exit.
+	 */
+	c->flags |= CLIENT_EXIT;
+
+	/* Does the target session still exist? */
+	if (session_index(s, &idx) != 0)
+		goto out;
+
+	psize = EVBUFFER_LENGTH(c->stdin_event->input);
+	if (psize == 0)
+		goto out;
+
+	pdata = malloc(psize + 1);
+	if (pdata == NULL)
+		goto out;
+	bufferevent_read(c->stdin_event, pdata, psize);
+	pdata[psize] = '\0';
+
+	limit = options_get_number(&s->options, "buffer-limit");
+	if (cdata->buffer == -1) {
+		paste_add(&s->buffers, pdata, psize, limit);
+		goto out;
+	}
+	if (paste_replace(&s->buffers, cdata->buffer, pdata, psize) != 0) {
+		/* No context so can't use server_client_msg_error. */
+		evbuffer_add_printf(
+		    c->stderr_event->output, "no buffer %d\n", cdata->buffer);
+		bufferevent_enable(c->stderr_event, EV_WRITE);
+		goto out;
+	}
+
+out:
+	xfree(cdata);
+}

cmd-run-shell.c

@@ -1,4 +1,4 @@
-/* $Id: cmd-run-shell.c,v 1.8 2010-07-17 14:36:40 tcunha Exp $ */
+/* $Id: cmd-run-shell.c,v 1.9 2010-08-09 21:44:25 tcunha Exp $ */
 
 /*
  * Copyright (c) 2009 Tiago Cunha <me@tiagocunha.org>
@@ -131,13 +131,10 @@ cmd_run_shell_free(void *data)
 {
 	struct cmd_run_shell_data	*cdata = data;
 	struct cmd_ctx			*ctx = &cdata->ctx;
-	struct msg_exit_data		 exitdata;
 
 	if (ctx->cmdclient != NULL) {
 		ctx->cmdclient->references--;
-		exitdata.retcode = ctx->cmdclient->retcode;
-		server_write_client(
-		    ctx->cmdclient, MSG_EXIT, &exitdata, sizeof exitdata);
+		ctx->cmdclient->flags |= CLIENT_EXIT;
 	}
 	if (ctx->curclient != NULL)
 		ctx->curclient->references--;

cmd-save-buffer.c

@@ -1,4 +1,4 @@
-/* $Id: cmd-save-buffer.c,v 1.11 2010-07-02 02:52:13 tcunha Exp $ */
+/* $Id: cmd-save-buffer.c,v 1.12 2010-08-09 21:44:25 tcunha Exp $ */
 
 /*
  * Copyright (c) 2009 Tiago Cunha <me@tiagocunha.org>
@@ -47,8 +47,8 @@ cmd_save_buffer_exec(struct cmd *self, struct cmd_ctx *ctx)
 	struct cmd_buffer_data	*data = self->data;
 	struct session		*s;
 	struct paste_buffer	*pb;
-	mode_t			mask;
-	FILE			*f, *close_f;
+	mode_t			 mask;
+	FILE			*f;
 
 	if ((s = cmd_find_session(ctx, data->target)) == NULL)
 		return (-1);
@@ -70,8 +70,8 @@ cmd_save_buffer_exec(struct cmd *self, struct cmd_ctx *ctx)
 			ctx->error(ctx, "%s: can't write to stdout", data->arg);
 			return (-1);
 		}
-		f = ctx->cmdclient->stdout_file;
-		close_f = NULL;
+		bufferevent_write(
+		    ctx->cmdclient->stdout_event, pb->data, pb->size);
 	} else {
 		mask = umask(S_IRWXG | S_IRWXO);
 		if (cmd_check_flag(data->chflags, 'a'))
@@ -83,17 +83,13 @@ cmd_save_buffer_exec(struct cmd *self, struct cmd_ctx *ctx)
 			ctx->error(ctx, "%s: %s", data->arg, strerror(errno));
 			return (-1);
 		}
-		close_f = f;
-	}
-
-	if (fwrite(pb->data, 1, pb->size, f) != pb->size) {
-	    	ctx->error(ctx, "%s: fwrite error", data->arg);
-	    	fclose(f);
-	    	return (-1);
+		if (fwrite(pb->data, 1, pb->size, f) != pb->size) {
+			ctx->error(ctx, "%s: fwrite error", data->arg);
+			fclose(f);
+			return (-1);
+		}
+		fclose(f);
 	}
 
-	if (close_f != NULL)
-		fclose(close_f);
-
 	return (0);
 }