Skip to content

Commit

Permalink
libsubprocess: increase output watcher priority
Browse files Browse the repository at this point in the history
Problem: If a output buffer is full, we issue an emergency call
to the user's output callback to empty the buffer before more
data is put in it.  This is done because we cannot control the
order in which check callbacks are called.  Now that check
callbacks can have priority set, we can ensure output check
callbacks are called first before a check callback that may put
more data in the buffer.

Set the priority of the output check watcher higher than the
default.  Remove the now unnecessary emergency callback to the
user's output callback.

Fixes flux-framework#6302
  • Loading branch information
chu11 committed Sep 26, 2024
1 parent 6aca2fe commit f124c7e
Showing 1 changed file with 7 additions and 8 deletions.
15 changes: 7 additions & 8 deletions src/common/libsubprocess/remote.c
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,13 @@ static int remote_channel_setup (flux_subprocess_t *p,
llog_debug (p, "flux_check_watcher_create: %s", strerror (errno));
goto error;
}

/* the output check should be called before other check
* callbacks, to ensure that the output buffer is emptied
* before any check callbacks that may move data into the
* buffer. So we up the priority of the output check
* watcher.
*/
flux_watcher_set_priority (c->out_check_w, 1);
/* don't start these watchers until we've reached the running
* state */
}
Expand Down Expand Up @@ -442,13 +448,6 @@ static int remote_output_buffered (flux_subprocess_t *p,
if (data && len) {
int tmp;

/* In the event the buffer is full, the `fbuf_write()` will
* fail. Call user callback to give them a chance to empty
* buffer. If they don't, we'll hit error below.
*/
if (!fbuf_space (c->read_buffer))
c->output_cb (c->p, c->name);

tmp = fbuf_write (c->read_buffer, data, len);
if (tmp >= 0 && tmp < len) {
errno = ENOSPC; // short write is promoted to fatal error
Expand Down

0 comments on commit f124c7e

Please sign in to comment.