diff --git a/src/common/libsubprocess/fbuf.h b/src/common/libsubprocess/fbuf.h index ec7cdce78558..9462ac6ae881 100644 --- a/src/common/libsubprocess/fbuf.h +++ b/src/common/libsubprocess/fbuf.h @@ -22,7 +22,7 @@ void fbuf_destroy (void *fb); /* Returns the buffer size, set when fbuf_create () was called */ int fbuf_size (struct fbuf *fb); -/* Returns the number of bytes current stored in fbuf */ +/* Returns the number of bytes currently stored in fbuf */ int fbuf_bytes (struct fbuf *fb); /* Returns the number of bytes of space available in fbuf */ diff --git a/src/common/libsubprocess/fbuf_watcher.h b/src/common/libsubprocess/fbuf_watcher.h index 9ddf53e0c9ea..3463a5727fc9 100644 --- a/src/common/libsubprocess/fbuf_watcher.h +++ b/src/common/libsubprocess/fbuf_watcher.h @@ -18,8 +18,15 @@ enum { FBUF_WATCHER_LINE_BUFFER = 1, /* line buffer data before invoking callback */ }; -/* on eof, callback will be called with an empty buffer */ -/* if line buffered, second to last callback may not contain a full line */ +/* read watcher + * + * - data from fd copied into buffer + * - when data is available, triggers callback + * - on eof, callback will be called with an empty buffer + * - if line buffered, second to last callback may not contain a full line + * - users should read from the buffer or stop the watcher, to avoid + * excessive event loop iterations without progress + */ flux_watcher_t *fbuf_read_watcher_create (flux_reactor_t *r, int fd, int size, @@ -40,7 +47,11 @@ const char *fbuf_read_watcher_get_data (flux_watcher_t *w, int *lenp); void fbuf_read_watcher_incref (flux_watcher_t *w); void fbuf_read_watcher_decref (flux_watcher_t *w); -/* 'cb' only called after fd closed (FLUX_POLLOUT) or error (FLUX_POLLERR) */ +/* write watcher + * + * - data from buffer written to fd + * - callback triggered after fd closed (FLUX_POLLOUT) or error (FLUX_POLLERR) + */ flux_watcher_t *fbuf_write_watcher_create (flux_reactor_t *r, int fd, int size, diff --git a/src/common/libsubprocess/local.c b/src/common/libsubprocess/local.c index 40a484284f51..8c5b47f581d0 100644 --- a/src/common/libsubprocess/local.c +++ b/src/common/libsubprocess/local.c @@ -154,14 +154,14 @@ static void local_out_cb (flux_reactor_t *r, } static void local_stdout_cb (flux_reactor_t *r, flux_watcher_t *w, - int revents, void *arg) + int revents, void *arg) { struct subprocess_channel *c = (struct subprocess_channel *)arg; local_output (c, w, revents, c->p->ops.on_stdout); } static void local_stderr_cb (flux_reactor_t *r, flux_watcher_t *w, - int revents, void *arg) + int revents, void *arg) { struct subprocess_channel *c = (struct subprocess_channel *)arg; local_output (c, w, revents, c->p->ops.on_stderr); diff --git a/src/common/libsubprocess/subprocess.h b/src/common/libsubprocess/subprocess.h index 98712e5f7353..a0125f3d0677 100644 --- a/src/common/libsubprocess/subprocess.h +++ b/src/common/libsubprocess/subprocess.h @@ -88,6 +88,11 @@ typedef void (*flux_subprocess_hook_f) (flux_subprocess_t *p, void *arg); /* * Functions for event-driven subprocess handling: * + * When output callbacks are called, flux_subprocess_read(), + * flux_subprocess_read_line() and similar functions should be used + * to read buffered data. If this is not done, it can lead to + * excessive callbacks and code "spinning". + * */ typedef struct { flux_subprocess_f on_completion; /* Process exited and all I/O