From 3d7b8105e1385c835cf0660797fe70198097e73d Mon Sep 17 00:00:00 2001 From: Tiago Cunha Date: Tue, 15 Feb 2011 15:20:03 +0000 Subject: [PATCH] Sync OpenBSD patchset 855: 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 | 8 +- cmd-run-shell.c | 8 +- cmd-server-info.c | 6 +- cmd-set-option.c | 25 +----- job.c | 211 +++++++++++++++------------------------------- server-client.c | 12 +-- status.c | 126 ++++++++++++++++++++------- tmux.h | 35 ++++---- 8 files changed, 201 insertions(+), 230 deletions(-) diff --git a/cmd-if-shell.c b/cmd-if-shell.c index 829102d9..d9dc3877 100644 --- a/cmd-if-shell.c +++ b/cmd-if-shell.c @@ -1,4 +1,4 @@ -/* $Id: cmd-if-shell.c,v 1.11 2011-01-07 14:45:34 tcunha Exp $ */ +/* $Id: cmd-if-shell.c,v 1.12 2011-02-15 15:20:03 tcunha Exp $ */ /* * Copyright (c) 2009 Tiago Cunha @@ -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, - args->argv[0], cmd_if_shell_callback, cmd_if_shell_free, cdata); - job_run(job); + job_run(shellcmd, cmd_if_shell_callback, cmd_if_shell_free, cdata); return (1); /* don't let client exit */ } diff --git a/cmd-run-shell.c b/cmd-run-shell.c index 09d0bd38..a763fa8c 100644 --- a/cmd-run-shell.c +++ b/cmd-run-shell.c @@ -1,4 +1,4 @@ -/* $Id: cmd-run-shell.c,v 1.10 2011-01-07 14:45:34 tcunha Exp $ */ +/* $Id: cmd-run-shell.c,v 1.11 2011-02-15 15:20:03 tcunha Exp $ */ /* * Copyright (c) 2009 Tiago Cunha @@ -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, - args->argv[0], cmd_run_shell_callback, cmd_run_shell_free, cdata); - job_run(job); + job_run(shellcmd, cmd_run_shell_callback, cmd_run_shell_free, cdata); return (1); /* don't let client exit */ } diff --git a/cmd-server-info.c b/cmd-server-info.c index 58a8de90..f09ba989 100644 --- a/cmd-server-info.c +++ b/cmd-server-info.c @@ -1,4 +1,4 @@ -/* $Id: cmd-server-info.c,v 1.43 2011-02-15 15:12:28 tcunha Exp $ */ +/* $Id: cmd-server-info.c,v 1.44 2011-02-15 15:20:03 tcunha Exp $ */ /* * Copyright (c) 2008 Nicholas Marriott @@ -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]", - job->cmd, job->fd, job->pid, job->status, job->flags); + ctx->print(ctx, "%s [fd=%d, pid=%d, status=%d]", + job->cmd, job->fd, job->pid, job->status); } return (0); diff --git a/cmd-set-option.c b/cmd-set-option.c index 88922db7..fbf08469 100644 --- a/cmd-set-option.c +++ b/cmd-set-option.c @@ -1,4 +1,4 @@ -/* $Id: cmd-set-option.c,v 1.107 2011-01-07 15:02:38 tcunha Exp $ */ +/* $Id: cmd-set-option.c,v 1.108 2011-02-15 15:20:03 tcunha Exp $ */ /* * Copyright (c) 2007 Nicholas Marriott @@ -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) - continue; - - 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); + if (c != NULL && c->session != NULL) + server_redraw_client(c); } } diff --git a/job.c b/job.c index 996bbdd4..3746f22c 100644 --- a/job.c +++ b/job.c @@ -1,4 +1,4 @@ -/* $Id: job.c,v 1.22 2011-02-15 15:12:28 tcunha Exp $ */ +/* $Id: job.c,v 1.23 2011-02-15 15:20:03 tcunha Exp $ */ /* * Copyright (c) 2009 Nicholas Marriott @@ -30,129 +30,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); - -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. */ +/* Start a job running, if it isn't already. */ 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, int flags, struct client *c, const char *cmd, +job_run(const char *cmd, void (*callbackfn)(struct job *), void (*freefn)(void *), void *data) { struct job *job; - - job = xmalloc(sizeof *job); - job->cmd = xstrdup(cmd); - 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); + struct environ env; + pid_t pid; + int nullfd, out[2]; 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); @@ -177,23 +80,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. */ @@ -203,15 +138,16 @@ job_callback(unused struct bufferevent *bufev, unused short events, void *data) { struct job *job = data; - bufferevent_disable(job->event, EV_READ); - close(job->fd); - job->fd = -1; + log_debug("job error %p: %s, pid %ld", job, job->cmd, (long) job->pid); 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; } } @@ -219,23 +155,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); - } -} - -/* Kill a job. */ -void -job_kill(struct job *job) -{ - if (job->pid == -1) - return; - kill(job->pid, SIGTERM); - job->pid = -1; + job_free(job); + } else + job->pid = -1; } diff --git a/server-client.c b/server-client.c index 93ba0fe9..e4ca3160 100644 --- a/server-client.c +++ b/server-client.c @@ -1,4 +1,4 @@ -/* $Id: server-client.c,v 1.53 2011-01-21 23:56:53 tcunha Exp $ */ +/* $Id: server-client.c,v 1.54 2011-02-15 15:20:03 tcunha Exp $ */ /* * Copyright (c) 2009 Nicholas Marriott @@ -78,7 +78,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); @@ -138,8 +139,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); @@ -220,7 +222,6 @@ server_client_status_timer(void) { struct client *c; struct session *s; - struct job *job; struct timeval tv; u_int i; int interval; @@ -249,8 +250,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) - job_run(job); + status_update_jobs(c); c->flags |= CLIENT_STATUS; } } diff --git a/status.c b/status.c index 9c29c818..7d29cad6 100644 --- a/status.c +++ b/status.c @@ -1,4 +1,4 @@ -/* $Id: status.c,v 1.155 2011-01-07 16:55:40 tcunha Exp $ */ +/* $Id: status.c,v 1.156 2011-02-15 15:20:03 tcunha Exp $ */ /* * Copyright (c) 2007 Nicholas Marriott @@ -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; - char *cmd; - int lastesc; - size_t len; + struct status_out *so, so_find; + char *cmd; + int lastesc; + 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); - if (job == NULL) { - job = job_add(&c->status_jobs, - JOB_PERSIST, c, cmd, status_job_callback, xfree, NULL); - job_run(job); + /* First try in the new tree. */ + so_find.cmd = cmd; + so = RB_FIND(status_out_tree, &c->status_new, &so_find); + if (so != NULL && so->out != NULL) + 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) - return (xstrdup("")); - return (xstrdup(job->data)); + if (so != NULL) + return (so->out); + 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; - size_t len; + struct client *c = job->data; + 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) - xfree(job->data); - else - server_redraw_client(job->client); - - if (line == NULL) - job->data = buf; - else - job->data = xstrdup(line); + so->out = buf; + server_redraw_client(c); } /* Return winlink status line entry and adjust gc as necessary. */ diff --git a/tmux.h b/tmux.h index a5f497fc..1ba85ea8 100644 --- a/tmux.h +++ b/tmux.h @@ -1,4 +1,4 @@ -/* $Id: tmux.h,v 1.609 2011-02-15 15:12:28 tcunha Exp $ */ +/* $Id: tmux.h,v 1.610 2011-02-15 15:20:03 tcunha Exp $ */ /* * Copyright (c) 2007 Nicholas Marriott @@ -667,8 +667,6 @@ struct job { pid_t pid; int status; - struct client *client; - int fd; struct bufferevent *event; @@ -676,13 +674,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. */ @@ -1087,6 +1080,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; @@ -1116,8 +1118,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 @@ -1359,18 +1362,10 @@ const char *options_table_print_entry( /* job.c */ extern struct joblist all_jobs; -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 *); -struct job *job_get(struct jobs *, const char *); -struct job *job_add(struct jobs *, int, struct client *, +struct job *job_run( 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 *); @@ -1656,6 +1651,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);