Use non-blocking I/O on non-SFTP session to avoid unnecessary waiting for data during reads. With multiple threads, libssh2 reads will call poll() to wait for available data, one read will handle ALL data, while the other read comes up dry.

src/protocols/ssh/client.h

@@ -48,6 +48,10 @@
 #include "sftp.h"
 #include "ssh_key.h"
 
+#ifdef ENABLE_SSH_AGENT
+#include "ssh_agent.h"
+#endif
+
 /**
  * SSH-specific client data.
  */
@@ -108,6 +112,11 @@ typedef struct ssh_guac_client_data {
      * Whether the SSH agent is enabled.
      */
     bool enable_agent;
+
+    /**
+     * The current agent, if any.
+     */
+    ssh_auth_agent* auth_agent;
 #endif
 
     /**

src/protocols/ssh/ssh_agent.c

@@ -88,8 +88,6 @@ void ssh_auth_agent_sign(ssh_auth_agent* agent, char* data, int data_length) {
     libssh2_channel_write(channel, buffer, pos-buffer);
     libssh2_channel_flush(channel);
 
-    usleep(10000);
-
 }
 
 void ssh_auth_agent_list_identities(ssh_auth_agent* auth_agent) {
@@ -113,8 +111,6 @@ void ssh_auth_agent_list_identities(ssh_auth_agent* auth_agent) {
     libssh2_channel_write(channel, buffer, pos-buffer);
     libssh2_channel_flush(channel);
 
-    usleep(10000);
-
 }
 
 void ssh_auth_agent_handle_packet(ssh_auth_agent* auth_agent, uint8_t type,
@@ -154,70 +150,52 @@ void ssh_auth_agent_handle_packet(ssh_auth_agent* auth_agent, uint8_t type,
 
 }
 
-void* ssh_auth_agent_read_thread(void* arg) {
+int ssh_auth_agent_read(ssh_auth_agent* auth_agent) {
 
-    ssh_auth_agent* auth_agent = (ssh_auth_agent*) arg;
     LIBSSH2_CHANNEL* channel = auth_agent->channel;
 
     int bytes_read;
-    char buffer[4096];
-    int buffer_length = 0;
 
-    /* Wait for channel to settle */
+    if (libssh2_channel_eof(channel))
-    usleep(10000);
+        return -1;
 
-    do {
+    /* Read header if available */
+    if (auth_agent->buffer_length >= 5) {
 
-        /* Read data into buffer */
-        while ((bytes_read = libssh2_channel_read(channel, buffer+buffer_length,
-                        sizeof(buffer)-buffer_length)) >= 0
-                || bytes_read == LIBSSH2_ERROR_EAGAIN) {
-
-            /* If re-read required, wait a bit and retry */
-            if (bytes_read == LIBSSH2_ERROR_EAGAIN) {
-                usleep(10000);
-                continue;
-            }
-
-            /* Update buffer length */
-            buffer_length += bytes_read;
-
-            /* Read length and type if given */
-            if (buffer_length >= 5) {
-
-                /* Read length */
         uint32_t length =
-                      (((unsigned char*) buffer)[0] << 24)
+              (((unsigned char*) auth_agent->buffer)[0] << 24)
-                    | (((unsigned char*) buffer)[1] << 16)
+            | (((unsigned char*) auth_agent->buffer)[1] << 16)
-                    | (((unsigned char*) buffer)[2] <<  8)
+            | (((unsigned char*) auth_agent->buffer)[2] <<  8)
-                    |  ((unsigned char*) buffer)[3];
+            |  ((unsigned char*) auth_agent->buffer)[3];
 
-                /* Read type */
+        uint8_t type = ((unsigned char*) auth_agent->buffer)[4];
-                uint8_t type = ((unsigned char*) buffer)[4];
 
         /* If enough data read, call handler, shift data */
-                if (buffer_length >= length+4) {
+        if (auth_agent->buffer_length >= length+4) {
 
             ssh_auth_agent_handle_packet(auth_agent, type,
-                            buffer+5, length-1);
+                    auth_agent->buffer+5, length-1);
 
-                    buffer_length -= length+4;
+            auth_agent->buffer_length -= length+4;
-                    memmove(buffer, buffer+length+4, buffer_length);
+            memmove(auth_agent->buffer,
+                    auth_agent->buffer+length+4, auth_agent->buffer_length);
+            return length+4;
+        }
 
     }
 
-            } /* end if have length and type */
+    /* Read data into buffer */
+    bytes_read = libssh2_channel_read(channel,
+            auth_agent->buffer+auth_agent->buffer_length,
+            sizeof(auth_agent->buffer)-auth_agent->buffer_length);
 
-            /* If EOF, stop now */
+    /* If unsuccessful, return error */
-            if (libssh2_channel_eof(channel))
+    if (bytes_read < 0)
-                break;
+        return bytes_read;
 
-        } /* end packet fill */
+    /* Update buffer length */
-
+    auth_agent->buffer_length += bytes_read;
-    } while (bytes_read >= 0 && !libssh2_channel_eof(channel));
+    return bytes_read;
-
-    /* Done */
-    return NULL;
 
 }
 
@@ -228,15 +206,14 @@ void ssh_auth_agent_callback(LIBSSH2_SESSION *session,
     guac_client* client = (guac_client*) *abstract;
     ssh_guac_client_data* client_data = (ssh_guac_client_data*) client->data;
 
-    /* Get base auth agent thread */
+    /* Init auth agent */
-    pthread_t read_thread;
     ssh_auth_agent* auth_agent = malloc(sizeof(ssh_auth_agent));
-
     auth_agent->channel = channel;
     auth_agent->identity = client_data->key;
+    auth_agent->buffer_length = 0;
 
-    /* Create thread */
+    /* Store auth agent */
-    pthread_create(&read_thread, NULL, ssh_auth_agent_read_thread, auth_agent);
+    client_data->auth_agent = auth_agent;
 
 }
 

src/protocols/ssh/ssh_agent.h

@@ -85,6 +85,16 @@ typedef struct ssh_auth_agent {
      */
     ssh_key* identity;
 
+    /**
+     * Data read from the agent channel.
+     */
+    char buffer[4096];
+
+    /**
+     * The number of bytes of data currently stored in the buffer.
+     */
+    int buffer_length;
+
 } ssh_auth_agent;
 
 /**
@@ -105,9 +115,11 @@ void ssh_auth_agent_handle_packet(ssh_auth_agent* auth_agent,
         uint8_t type, char* data, int data_length);
 
 /**
- * Auth agent channel thread.
+ * Reads and handles a single packet from the SSH agent channel associated
+ * with the given ssh_auth_agent, returning the size of that packet, the size
+ * of the partial packet read, or a negative value if an error occurs.
  */
-void* auth_agent_read_thread(void* arg);
+int ssh_auth_agent_read(ssh_auth_agent* auth_agent);
 
 /**
  * Libssh2 callback, invoked when the auth agent channel is opened.

src/protocols/ssh/ssh_client.c

@@ -159,7 +159,8 @@ static int __sign_callback(LIBSSH2_SESSION* session,
 }
 
 
-static LIBSSH2_SESSION* __guac_ssh_create_session(guac_client* client) {
+static LIBSSH2_SESSION* __guac_ssh_create_session(guac_client* client,
+        int* socket_fd) {
 
     int retval;
 
@@ -252,6 +253,10 @@ static LIBSSH2_SESSION* __guac_ssh_create_session(guac_client* client) {
         return NULL;
     }
 
+    /* Save file descriptor */
+    if (socket_fd != NULL)
+        *socket_fd = fd;
+
     /* Authenticate with key if available */
     if (client_data->key != NULL) {
         if (!libssh2_userauth_publickey(session, client_data->username,
@@ -294,6 +299,7 @@ void* ssh_client_thread(void* data) {
     char buffer[8192];
     int bytes_read = -1234;
 
+    int socket_fd;
     int stdout_fd = client_data->term->stdout_pipe_fd[1];
 
     pthread_t input_thread;
@@ -356,7 +362,7 @@ void* ssh_client_thread(void* data) {
     guac_terminal_write_all(stdout_fd, "\x1B[H\x1B[J", 6);
 
     /* Open SSH session */
-    client_data->session = __guac_ssh_create_session(client);
+    client_data->session = __guac_ssh_create_session(client, &socket_fd);
     if (client_data->session == NULL) {
         guac_protocol_send_error(socket, "Unable to create SSH session.",
                 GUAC_PROTOCOL_STATUS_INTERNAL_ERROR);
@@ -386,6 +392,8 @@ void* ssh_client_thread(void* data) {
         else
             guac_client_log_info(client, "Agent forwarding enabled.");
     }
+
+    client_data->auth_agent = NULL;
 #endif
 
     /* Start SFTP session as well, if enabled */
@@ -393,7 +401,7 @@ void* ssh_client_thread(void* data) {
 
         /* Create SSH session specific for SFTP */
         guac_client_log_info(client, "Reconnecting for SFTP...");
-        client_data->sftp_ssh_session = __guac_ssh_create_session(client);
+        client_data->sftp_ssh_session = __guac_ssh_create_session(client, NULL);
 
         /* Request SFTP */
         client_data->sftp_session =
@@ -443,18 +451,52 @@ void* ssh_client_thread(void* data) {
         return NULL;
     }
 
+    /* Set non-blocking */
+    libssh2_session_set_blocking(client_data->session, 0);
+
     /* While data available, write to terminal */
+    bytes_read = 0;
     while (!libssh2_channel_eof(client_data->term_channel)) {
 
-        /* Repeat read if necessary */
+        /* Track total amount of data read */
-        if ((bytes_read = libssh2_channel_read(client_data->term_channel, buffer, sizeof(buffer))) == LIBSSH2_ERROR_EAGAIN)
+        int total_read = 0;
-            continue;
+
+        /* Read terminal data */
+        bytes_read = libssh2_channel_read(client_data->term_channel,
+                buffer, sizeof(buffer));
 
         /* Attempt to write data received. Exit on failure. */
         if (bytes_read > 0) {
             int written = guac_terminal_write_all(stdout_fd, buffer, bytes_read);
             if (written < 0)
                 break;
+
+            total_read += bytes_read;
+        }
+
+        /* If agent open, handle any agent packets */
+        if (client_data->auth_agent != NULL) {
+            bytes_read = ssh_auth_agent_read(client_data->auth_agent);
+            if (bytes_read > 0)
+                total_read += bytes_read;
+            else if (bytes_read < 0 && bytes_read != LIBSSH2_ERROR_EAGAIN)
+                client_data->auth_agent = NULL;
+        }
+
+        /* Wait for more data if reads turn up empty */
+        if (total_read == 0) {
+            fd_set fds;
+            struct timeval timeout;
+
+            FD_ZERO(&fds);
+            FD_SET(socket_fd, &fds);
+
+            /* Wait for one second */
+            timeout.tv_sec = 1;
+            timeout.tv_usec = 0;
+
+            if (select(socket_fd+1, &fds, NULL, NULL, &timeout) < 0)
+                break;
         }
 
     }