Skip to content

Commit

Permalink
Handle window resize events in a different fifo
Browse files Browse the repository at this point in the history 
The terminal_ctrl_fd was being overloaded with events. CRI-O wants to reopen the log files at any time, and podman wants to not drop window resize events on exec.

But we can have both.

Introduce winsz_fd_*, a special fifo pair for window resize events. The terminal_ctrl passes window resize events to the write side, and the read side can process them (after masterfd_stdout is initialized, so we have somewhere to write to and we don't drop events).

Signed-off-by: Peter Hunt <[email protected]>
  • Loading branch information
@haircommander
haircommander committed Nov 8, 2019
1 parent 002da25 commit 1ed4f02
Showing 1 changed file with 113 additions and 42 deletions.
155 changes: 113 additions & 42 deletions src/conmon.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ static ssize_t write_all(int fd, const void *buf, size_t count)
return count;
}


/*
* Returns the path for specified controller name for a pid.
* Returns NULL on error.
Expand Down Expand Up @@ -292,6 +291,8 @@ static int attach_socket_fd = -1;
static int console_socket_fd = -1;
static int terminal_ctrl_fd = -1;
static int inotify_fd = -1;
static int winsz_fd_w = -1;
static int winsz_fd_r = -1;

static gboolean timed_out = FALSE;

Expand Down Expand Up @@ -683,48 +684,35 @@ static void resize_winsz(int height, int width)
}

#define CTLBUFSZ 200
static gboolean ctrl_cb(int fd, G_GNUC_UNUSED GIOCondition condition, G_GNUC_UNUSED gpointer user_data)
/*
* read_from_ctrl_buffer reads a line (of no more than CTLBUFSZ) from an fd,
* and calls line_process_func. It is a generic way to handle input on an fd
* line_process_func should return TRUE if it succeeds, and FALSE if it fails
* to process the line.
*/
static gboolean read_from_ctrl_buffer(int fd, gboolean(*line_process_func)(char*,int))
{
static char ctlbuf[CTLBUFSZ];
static int readsz = CTLBUFSZ - 1;
static char *readptr = ctlbuf;
ssize_t num_read = 0;
int ctl_msg_type = -1;
int height = -1;
int width = -1;
int ret;

num_read = read(fd, readptr, readsz);
if (num_read <= 0) {
nwarn("Failed to read from control fd");
nwarnf("Failed to read from fd %d", fd);
return G_SOURCE_CONTINUE;
}

readptr[num_read] = '\0';
ninfof("Got ctl message: %s", ctlbuf);
ninfof("Got ctl message: %s on fd %d", ctlbuf, fd);

char *beg = ctlbuf;
char *newline = strchrnul(beg, '\n');
/* Process each message which ends with a line */
while (*newline != '\0') {
ret = sscanf(ctlbuf, "%d %d %d\n", &ctl_msg_type, &height, &width);
if (ret != 3) {
nwarn("Failed to sscanf message");
if (!line_process_func(ctlbuf, num_read)) {
return G_SOURCE_CONTINUE;
}
ninfof("Message type: %d, Height: %d, Width: %d", ctl_msg_type, height, width);
switch (ctl_msg_type) {
// This matches what we write from container_attach.go
case 1:
resize_winsz(height, width);
break;
case 2:
reopen_log_files();
break;
default:
ninfof("Unknown message type: %d", ctl_msg_type);
break;
}
beg = newline + 1;
newline = strchrnul(beg, '\n');
}
Expand Down Expand Up @@ -755,6 +743,79 @@ static gboolean ctrl_cb(int fd, G_GNUC_UNUSED GIOCondition condition, G_GNUC_UNU
return G_SOURCE_CONTINUE;
}

/*
* process_terminal_ctrl_line takes a line from the
* caller program (received through the terminal ctrl fd)
* and either writes to the winsz fd (to handle terminal resize events)
* or reopens log files.
*/
static gboolean process_terminal_ctrl_line(char* line, int len)
{
int ctl_msg_type, height, width, ret = -1;

ret = sscanf(line, "%d %d %d\n", &ctl_msg_type, &height, &width);
if (ret != 3) {
nwarn("Failed to sscanf message");
return FALSE;
}

ninfof("Message type: %d, Height: %d, Width: %d", ctl_msg_type, height, width);
switch (ctl_msg_type) {
// This matches what we write from container_attach.go
case 1:
if (write(winsz_fd_w, line, len) < 0) {
nwarn("Failed to write to window resizing fd. A resize event may have been dropped");
return FALSE;
}
break;
case 2:
reopen_log_files();
break;
default:
ninfof("Unknown message type: %d", ctl_msg_type);
break;
}
return TRUE;
}

/*
* ctrl_cb is a callback for handling events directly from the caller
*/
static gboolean ctrl_cb(int fd, G_GNUC_UNUSED GIOCondition condition, G_GNUC_UNUSED gpointer user_data)
{
return read_from_ctrl_buffer(fd, process_terminal_ctrl_line);
}

/*
* process_winsz_ctrl_line processes a line passed to the winsz fd
* after the terminal_ctrl fd receives a winsz event.
* It reads a height and length, and resizes the pty with it.
*/
static gboolean process_winsz_ctrl_line(char * line, G_GNUC_UNUSED int len)
{
int ctl_msg_type, height, width, ret = -1;
ret = sscanf(line, "%d %d %d\n", &ctl_msg_type, &height, &width);
if (ret != 3) {
nwarn("Failed to sscanf message");
return FALSE;
}
if (ctl_msg_type != 1) {
ninfof("Unknown message type: %d", ctl_msg_type);
return FALSE;
}
ninfof("Message type: %d, Height: %d, Width: %d", ctl_msg_type, height, width);
resize_winsz(height, width);
return TRUE;
}

/*
* ctrl_winsz_cb is a callback after a window resize event is sent along the winsz fd.
*/
static gboolean ctrl_winsz_cb(int fd, G_GNUC_UNUSED GIOCondition condition, G_GNUC_UNUSED gpointer user_data)
{
return read_from_ctrl_buffer(fd, process_winsz_ctrl_line);
}

static gboolean terminal_accept_cb(int fd, G_GNUC_UNUSED GIOCondition condition, G_GNUC_UNUSED gpointer user_data)
{
const char *csname = user_data;
Expand Down Expand Up @@ -797,10 +858,10 @@ static gboolean terminal_accept_cb(int fd, G_GNUC_UNUSED GIOCondition condition,
masterfd_stdin = console.fd;
masterfd_stdout = console.fd;

/* now that we've set masterfd_stdout, we can register the ctrl_cb
/* now that we've set masterfd_stdout, we can register the ctrl_winsz_cb
* if we didn't set it here, we'd risk attempting to run ioctl on
* a negative fd, and fail to resize the window */
g_unix_fd_add(terminal_ctrl_fd, G_IO_IN, ctrl_cb, NULL);
g_unix_fd_add(winsz_fd_r, G_IO_IN, ctrl_winsz_cb, NULL);

/* Clean up everything */
close(connfd);
Expand Down Expand Up @@ -974,29 +1035,38 @@ static char *setup_attach_socket(void)
return attach_symlink_dir_path;
}

static int setup_terminal_control_fifo()
{
_cleanup_free_ char *ctl_fifo_path = g_build_filename(opt_bundle_path, "ctl", NULL);
ninfof("ctl fifo path: %s", ctl_fifo_path);
static void setup_fifo(int *fifo_r, int *fifo_w, char * filename, char* error_var_name) {
_cleanup_free_ char *fifo_path = g_build_filename(opt_bundle_path, filename, NULL);

if (!fifo_r || !fifo_w)
pexitf("setup fifo was passed a NULL pointer");

/* Setup fifo for reading in terminal resize and other stdio control messages */
if (mkfifo(fifo_path, 0666) == -1)
pexitf("Failed to mkfifo at %s", fifo_path);

if (mkfifo(ctl_fifo_path, 0666) == -1)
pexitf("Failed to mkfifo at %s", ctl_fifo_path);
if ((*fifo_r = open(fifo_path, O_RDONLY | O_NONBLOCK | O_CLOEXEC)) == -1)
pexitf("Failed to open %s read half", error_var_name);

terminal_ctrl_fd = open(ctl_fifo_path, O_RDONLY | O_NONBLOCK | O_CLOEXEC);
if (terminal_ctrl_fd == -1)
pexit("Failed to open control fifo");
if ((*fifo_w = open(fifo_path, O_WRONLY | O_CLOEXEC)) == -1)
pexitf("Failed to open %s write half", error_var_name);
}

static void setup_console_fifo() {
setup_fifo(&winsz_fd_r, &winsz_fd_w, "winsz", "window resize control fifo");
ninfof("winsz read side: %d, winsz write side: %d", winsz_fd_r, winsz_fd_r);
}

static int setup_terminal_control_fifo()
{
/*
* Open a dummy writer to prevent getting flood of POLLHUPs when
* last writer closes.
*/
int dummyfd = open(ctl_fifo_path, O_WRONLY | O_CLOEXEC);
if (dummyfd == -1)
pexit("Failed to open dummy writer for fifo");

int dummyfd = -1;
setup_fifo(&terminal_ctrl_fd, &dummyfd, "ctl", "terminal control fifo");
ninfof("terminal_ctrl_fd: %d", terminal_ctrl_fd);
g_unix_fd_add(terminal_ctrl_fd, G_IO_IN, ctrl_cb, NULL);

return dummyfd;
}

Expand Down Expand Up @@ -1326,10 +1396,10 @@ int main(int argc, char *argv[])
masterfd_stdout = fds[0];
slavefd_stdout = fds[1];

/* now that we've set masterfd_stdout, we can register the ctrl_cb
/* now that we've set masterfd_stdout, we can register the ctrl_winsz_cb
* if we didn't set it here, we'd risk attempting to run ioctl on
* a negative fd, and fail to resize the window */
g_unix_fd_add(terminal_ctrl_fd, G_IO_IN, ctrl_cb, NULL);
g_unix_fd_add(winsz_fd_r, G_IO_IN, ctrl_winsz_cb, NULL);
}

/* We always create a stderr pipe, because that way we can capture
Expand Down Expand Up @@ -1524,6 +1594,7 @@ int main(int argc, char *argv[])
if (opt_bundle_path != NULL) {
attach_symlink_dir_path = setup_attach_socket();
dummyfd = setup_terminal_control_fifo();
setup_console_fifo();

if (opt_attach) {
ndebug("sending attach message to parent");
Expand Down

0 comments on commit 1ed4f02

Please sign in to comment.