GUACAMOLE-86: Remove terminal emulator's STDOUT pipe. Use pthread_cond_t to signal modification.
diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c
index db3cea9..4bdf76f 100644
--- a/src/protocols/ssh/ssh.c
+++ b/src/protocols/ssh/ssh.c
@@ -339,7 +339,7 @@
/* Attempt to write data received. Exit on failure. */
if (bytes_read > 0) {
- int written = guac_terminal_write_stdout(ssh_client->term, buffer, bytes_read);
+ int written = guac_terminal_write(ssh_client->term, buffer, bytes_read);
if (written < 0)
break;
diff --git a/src/protocols/telnet/telnet.c b/src/protocols/telnet/telnet.c
index 6ca9aba..531819f 100644
--- a/src/protocols/telnet/telnet.c
+++ b/src/protocols/telnet/telnet.c
@@ -150,7 +150,7 @@
/* Terminal output received */
case TELNET_EV_DATA:
- guac_terminal_write_stdout(telnet_client->term, event->data.buffer, event->data.size);
+ guac_terminal_write(telnet_client->term, event->data.buffer, event->data.size);
/* Continue search for username prompt */
if (settings->username_regex != NULL) {
@@ -267,7 +267,7 @@
while ((bytes_read = guac_terminal_read_stdin(telnet_client->term, buffer, sizeof(buffer))) > 0) {
telnet_send(telnet_client->telnet, buffer, bytes_read);
if (telnet_client->echo_enabled)
- guac_terminal_write_stdout(telnet_client->term, buffer, bytes_read);
+ guac_terminal_write(telnet_client->term, buffer, bytes_read);
}
return NULL;
diff --git a/src/terminal/Makefile.am b/src/terminal/Makefile.am
index 19cbf94..59bcd12 100644
--- a/src/terminal/Makefile.am
+++ b/src/terminal/Makefile.am
@@ -27,7 +27,6 @@
char_mappings.h \
common.h \
display.h \
- packet.h \
scrollbar.h \
terminal.h \
terminal_handlers.h \
@@ -39,7 +38,6 @@
char_mappings.c \
common.c \
display.c \
- packet.c \
scrollbar.c \
terminal.c \
terminal_handlers.c \
diff --git a/src/terminal/common.c b/src/terminal/common.c
index cc98276..4018e77 100644
--- a/src/terminal/common.c
+++ b/src/terminal/common.c
@@ -105,23 +105,3 @@
}
-int guac_terminal_fill_buffer(int fd, char* buffer, int size) {
-
- int remaining = size;
- while (remaining > 0) {
-
- /* Attempt to read data */
- int ret_val = read(fd, buffer, remaining);
- if (ret_val <= 0)
- return -1;
-
- /* If successful, continue with what space remains (if any) */
- remaining -= ret_val;
- buffer += ret_val;
-
- }
-
- return size;
-
-}
-
diff --git a/src/terminal/common.h b/src/terminal/common.h
index 495bf30..f7eda61 100644
--- a/src/terminal/common.h
+++ b/src/terminal/common.h
@@ -49,26 +49,5 @@
*/
int guac_terminal_write_all(int fd, const char* buffer, int size);
-/**
- * Similar to read, but automatically retries the read until an error occurs,
- * filling all available space within the buffer. Unless it is known that the
- * given amount of space is available on the file descriptor, there is a good
- * chance this function will block.
- *
- * @param fd
- * The file descriptor to read data from.
- *
- * @param buffer
- * The buffer to store data within.
- *
- * @param size
- * The number of bytes available within the buffer.
- *
- * @return
- * The number of bytes read if successful, or a negative value if an error
- * occurs.
- */
-int guac_terminal_fill_buffer(int fd, char* buffer, int size);
-
#endif
diff --git a/src/terminal/packet.c b/src/terminal/packet.c
deleted file mode 100644
index ad828ce..0000000
--- a/src/terminal/packet.c
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-#include "common.h"
-#include "packet.h"
-
-#include <string.h>
-
-int guac_terminal_packet_write(int fd, const void* data, int length) {
-
- guac_terminal_packet out;
-
- /* Do not attempt to write packets beyond maximum size */
- if (length > GUAC_TERMINAL_PACKET_SIZE)
- return -1;
-
- /* Calculate final packet length */
- int packet_length = sizeof(int) + length;
-
- /* Copy data into packet */
- out.length = length;
- memcpy(out.data, data, length);
-
- /* Write packet */
- return guac_terminal_write_all(fd, (const char*) &out, packet_length);
-
-}
-
-int guac_terminal_packet_read(int fd, void* data, int length) {
-
- int bytes;
-
- /* Read buffers MUST be at least GUAC_TERMINAL_PACKET_SIZE */
- if (length < GUAC_TERMINAL_PACKET_SIZE)
- return -1;
-
- /* Read length */
- if (guac_terminal_fill_buffer(fd, (char*) &bytes, sizeof(int)) < 0)
- return -1;
-
- /* Read body */
- if (guac_terminal_fill_buffer(fd, (char*) data, bytes) < 0)
- return -1;
-
- return bytes;
-
-}
-
diff --git a/src/terminal/packet.h b/src/terminal/packet.h
deleted file mode 100644
index 00d3052..0000000
--- a/src/terminal/packet.h
+++ /dev/null
@@ -1,89 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-#ifndef GUAC_TERMINAL_PACKET_H
-#define GUAC_TERMINAL_PACKET_H
-
-/**
- * The maximum size of a packet written or read by the
- * guac_terminal_packet_write() or guac_terminal_packet_read() functions.
- */
-#define GUAC_TERMINAL_PACKET_SIZE 4096
-
-/**
- * An arbitrary data packet with minimal framing.
- */
-typedef struct guac_terminal_packet {
-
- /**
- * The number of bytes in the data portion of this packet.
- */
- int length;
-
- /**
- * Arbitrary data.
- */
- char data[GUAC_TERMINAL_PACKET_SIZE];
-
-} guac_terminal_packet;
-
-/**
- * Writes a single packet of data to the given file descriptor. The provided
- * length MUST be no greater than GUAC_TERMINAL_PACKET_SIZE. Zero-length
- * writes are legal and do result in a packet being written to the file
- * descriptor.
- *
- * @param fd
- * The file descriptor to write to.
- *
- * @param data
- * A buffer containing the data to write.
- *
- * @param length
- * The number of bytes to write to the file descriptor.
- *
- * @return
- * The number of bytes written on success, which may be zero if the data
- * length is zero, or a negative value on error.
- */
-int guac_terminal_packet_write(int fd, const void* data, int length);
-
-/**
- * Reads a single packet of data from the given file descriptor. The provided
- * length MUST be at least GUAC_TERMINAL_PACKET_SIZE to ensure any packet
- * read will fit in the buffer. Zero-length reads are possible if a zero-length
- * packet was written.
- *
- * @param fd
- * The file descriptor to read from.
- *
- * @param data
- * The buffer to store data within.
- *
- * @param length
- * The number of bytes available within the buffer.
- *
- * @return
- * The number of bytes read on success, which may be zero if the read
- * packet had a length of zero, or a negative value on error.
- */
-int guac_terminal_packet_read(int fd, void* data, int length);
-
-#endif
-
diff --git a/src/terminal/terminal.c b/src/terminal/terminal.c
index 598a385..7488a31 100644
--- a/src/terminal/terminal.c
+++ b/src/terminal/terminal.c
@@ -24,7 +24,6 @@
#include "display.h"
#include "guac_clipboard.h"
#include "guac_cursor.h"
-#include "packet.h"
#include "scrollbar.h"
#include "terminal.h"
#include "terminal_handlers.h"
@@ -32,7 +31,6 @@
#include "typescript.h"
#include <errno.h>
-#include <poll.h>
#include <pthread.h>
#include <stdarg.h>
#include <stdio.h>
@@ -318,6 +316,11 @@
term->upload_path_handler = NULL;
term->file_download_handler = NULL;
+ /* Init modified flag and conditional */
+ term->modified = 0;
+ pthread_cond_init(&(term->modified_cond), NULL);
+ pthread_mutex_init(&(term->modified_lock), NULL);
+
/* Init buffer */
term->buffer = guac_terminal_buffer_alloc(1000, &default_char);
@@ -349,14 +352,6 @@
term->term_width = available_width / term->display->char_width;
term->term_height = height / term->display->char_height;
- /* Open STDOUT pipe */
- if (pipe(term->stdout_pipe_fd)) {
- guac_error = GUAC_STATUS_SEE_ERRNO;
- guac_error_message = "Unable to open pipe for STDOUT";
- free(term);
- return NULL;
- }
-
/* Open STDIN pipe */
if (pipe(term->stdin_pipe_fd)) {
guac_error = GUAC_STATUS_SEE_ERRNO;
@@ -414,10 +409,6 @@
void guac_terminal_free(guac_terminal* term) {
- /* Close terminal output pipe */
- close(term->stdout_pipe_fd[1]);
- close(term->stdout_pipe_fd[0]);
-
/* Close user input pipe */
close(term->stdin_pipe_fd[1]);
close(term->stdin_pipe_fd[0]);
@@ -449,84 +440,110 @@
}
/**
- * Waits for data to become available on the given file descriptor.
+ * Populate the given timespec with the current time, plus the given offset.
*
- * @param fd
- * The file descriptor to wait on.
+ * @param ts
+ * The timespec structure to populate.
+ *
+ * @param offset_sec
+ * The offset from the current time to use when populating the given
+ * timespec, in seconds.
+ *
+ * @param offset_usec
+ * The offset from the current time to use when populating the given
+ * timespec, in microseconds.
+ */
+static void guac_terminal_get_absolute_time(struct timespec* ts,
+ int offset_sec, int offset_usec) {
+
+ /* Get timeval */
+ struct timeval tv;
+ gettimeofday(&tv, NULL);
+
+ /* Update with offset */
+ tv.tv_sec += offset_sec;
+ tv.tv_usec += offset_usec;
+
+ /* Wrap to next second if necessary */
+ if (tv.tv_usec >= 1000000) {
+ tv.tv_sec++;
+ tv.tv_usec -= 1000000;
+ }
+
+ /* Convert to timespec */
+ ts->tv_sec = tv.tv_sec;
+ ts->tv_nsec = tv.tv_usec * 1000;
+
+}
+
+/**
+ * Waits for the terminal state to be modified, returning only when the
+ * specified timeout has elapsed or a frame flush is desired. Note that the
+ * modified flag of the terminal will only be reset if no data remains to be
+ * read from STDOUT.
+ *
+ * @param terminal
+ * The terminal to wait on.
*
* @param msec_timeout
* The maximum amount of time to wait, in milliseconds.
*
* @return
- * A positive if data is available, zero if the timeout has elapsed without
- * data becoming available, or negative if an error occurred.
+ * Non-zero if the terminal has been modified, zero if the timeout has
+ * elapsed without the terminal being modified.
*/
-static int guac_terminal_wait_for_data(int fd, int msec_timeout) {
+static int guac_terminal_wait(guac_terminal* terminal, int msec_timeout) {
- /* Build array of file descriptors */
- struct pollfd fds[] = {{
- .fd = fd,
- .events = POLLIN,
- .revents = 0,
- }};
+ int retval = 1;
- /* Wait for data */
- return poll(fds, 1, msec_timeout);
+ pthread_mutex_t* mod_lock = &(terminal->modified_lock);
+ pthread_cond_t* mod_cond = &(terminal->modified_cond);
+
+ /* Split provided milliseconds into microseconds and whole seconds */
+ int secs = msec_timeout / 1000;
+ int usecs = (msec_timeout % 1000) * 1000;
+
+ /* Calculate absolute timestamp from provided relative timeout */
+ struct timespec timeout;
+ guac_terminal_get_absolute_time(&timeout, secs, usecs);
+
+ /* Test for terminal modification */
+ pthread_mutex_lock(mod_lock);
+ if (terminal->modified)
+ goto wait_complete;
+
+ /* If not yet modified, wait for modification condition to be signaled */
+ retval = pthread_cond_timedwait(mod_cond, mod_lock, &timeout) != ETIMEDOUT;
+
+wait_complete:
+
+ /* Terminal is no longer modified */
+ terminal->modified = 0;
+ pthread_mutex_unlock(mod_lock);
+ return retval;
}
int guac_terminal_render_frame(guac_terminal* terminal) {
- guac_client* client = terminal->client;
- char buffer[GUAC_TERMINAL_PACKET_SIZE];
-
int wait_result;
- int fd = terminal->stdout_pipe_fd[0];
/* Wait for data to be available */
- wait_result = guac_terminal_wait_for_data(fd, 1000);
- if (wait_result > 0) {
+ wait_result = guac_terminal_wait(terminal, 1000);
+ if (wait_result) {
- guac_terminal_lock(terminal);
guac_timestamp frame_start = guac_timestamp_current();
do {
- guac_timestamp frame_end;
- int frame_remaining;
-
- int bytes_read;
-
- /* Read data, write to terminal */
- if ((bytes_read = guac_terminal_packet_read(fd,
- buffer, sizeof(buffer))) > 0) {
-
- if (guac_terminal_write(terminal, buffer, bytes_read)) {
- guac_client_abort(client,
- GUAC_PROTOCOL_STATUS_SERVER_ERROR,
- "Error writing data");
- guac_terminal_unlock(terminal);
- return 1;
- }
-
- }
-
- /* Notify on error */
- if (bytes_read < 0) {
- guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR,
- "Error reading data");
- guac_terminal_unlock(terminal);
- return 1;
- }
-
/* Calculate time remaining in frame */
- frame_end = guac_timestamp_current();
- frame_remaining = frame_start + GUAC_TERMINAL_FRAME_DURATION
- - frame_end;
+ guac_timestamp frame_end = guac_timestamp_current();
+ int frame_remaining = frame_start + GUAC_TERMINAL_FRAME_DURATION
+ - frame_end;
/* Wait again if frame remaining */
if (frame_remaining > 0)
- wait_result = guac_terminal_wait_for_data(fd,
+ wait_result = guac_terminal_wait(terminal,
GUAC_TERMINAL_FRAME_TIMEOUT);
else
break;
@@ -534,18 +551,12 @@
} while (wait_result > 0);
/* Flush terminal */
+ guac_terminal_lock(terminal);
guac_terminal_flush(terminal);
guac_terminal_unlock(terminal);
}
- /* Notify of any errors */
- if (wait_result < 0) {
- guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR,
- "Error waiting for data");
- return 1;
- }
-
return 0;
}
@@ -555,28 +566,19 @@
return read(stdin_fd, c, size);
}
-int guac_terminal_write_stdout(guac_terminal* terminal, const char* c,
- int size) {
+void guac_terminal_notify(guac_terminal* terminal) {
- /* Write maximally-sized packets until only one packet remains */
- while (size > GUAC_TERMINAL_PACKET_SIZE) {
+ pthread_mutex_t* mod_lock = &(terminal->modified_lock);
+ pthread_cond_t* mod_cond = &(terminal->modified_cond);
- /* Write maximally-sized packet */
- if (guac_terminal_packet_write(terminal->stdout_pipe_fd[1], c,
- GUAC_TERMINAL_PACKET_SIZE) < 0)
- return -1;
+ pthread_mutex_lock(mod_lock);
- /* Advance to next packet */
- c += GUAC_TERMINAL_PACKET_SIZE;
- size -= GUAC_TERMINAL_PACKET_SIZE;
+ /* Signal modification */
+ terminal->modified = 1;
+ pthread_cond_signal(mod_cond);
- }
+ pthread_mutex_unlock(mod_lock);
- return guac_terminal_packet_write(terminal->stdout_pipe_fd[1], c, size);
-}
-
-int guac_terminal_notify(guac_terminal* terminal) {
- return guac_terminal_packet_write(terminal->stdout_pipe_fd[1], NULL, 0);
}
int guac_terminal_printf(guac_terminal* terminal, const char* format, ...) {
@@ -595,7 +597,7 @@
return written;
/* Write to STDOUT */
- return guac_terminal_write_stdout(terminal, buffer, written);
+ return guac_terminal_write(terminal, buffer, written);
}
@@ -711,6 +713,7 @@
int guac_terminal_write(guac_terminal* term, const char* c, int size) {
+ guac_terminal_lock(term);
while (size > 0) {
/* Read and advance to next character */
@@ -725,7 +728,9 @@
term->char_handler(term, current);
}
+ guac_terminal_unlock(term);
+ guac_terminal_notify(term);
return 0;
}
diff --git a/src/terminal/terminal.h b/src/terminal/terminal.h
index 7598fce..62312b0 100644
--- a/src/terminal/terminal.h
+++ b/src/terminal/terminal.h
@@ -159,13 +159,24 @@
pthread_mutex_t lock;
/**
- * Pipe which should be written to (and read from) to provide output to
- * this terminal. Another thread should read from this pipe when writing
- * data to the terminal. It would make sense for the terminal to provide
- * this thread, but for simplicity, that logic is left to the guac
- * message handler (to give the message handler something to block with).
+ * The mutex associated with the modified condition and flag, locked
+ * whenever a thread is waiting on the modified condition, the modified
+ * condition is being signalled, or the modified flag is being changed.
*/
- int stdout_pipe_fd[2];
+ pthread_mutex_t modified_lock;
+
+ /**
+ * Flag set whenever an operation has affected the terminal in a way that
+ * will require a frame flush. When this flag is set, the modified_cond
+ * condition will be signalled. The modified_lock will always be
+ * acquired before this flag is altered.
+ */
+ int modified;
+
+ /**
+ * Condition which is signalled when the modified flag has been set
+ */
+ pthread_cond_t modified_cond;
/**
* Pipe which will be the source of user input. When a terminal code
@@ -474,23 +485,13 @@
int guac_terminal_read_stdin(guac_terminal* terminal, char* c, int size);
/**
- * Writes to this terminal's STDOUT. This function may block until space
- * is freed in the output buffer by guac_terminal_render_frame().
- */
-int guac_terminal_write_stdout(guac_terminal* terminal, const char* c, int size);
-
-/**
* Notifies the terminal that an event has occurred and the terminal should
* flush itself when reasonable.
*
* @param terminal
* The terminal to notify.
- *
- * @return
- * Zero if notification succeeded, non-zero if an error occurred while
- * notifying the terminal.
*/
-int guac_terminal_notify(guac_terminal* terminal);
+void guac_terminal_notify(guac_terminal* terminal);
/**
* Reads a single line from this terminal's STDIN, storing the result in a
