Improved instruction handling, working I/O threads.

libguac/Makefile.am

@@ -44,7 +44,7 @@ libguacinc_HEADERS = include/client.h include/guacio.h include/protocol.h includ
 
 lib_LTLIBRARIES = libguac.la
 
-libguac_la_SOURCES = src/client.c src/guacio.c src/protocol.c
+libguac_la_SOURCES = src/client.c src/guacio.c src/protocol.c src/client-handlers.c
 
 libguac_la_LDFLAGS = -version-info 0:0:0
 

libguac/include/client-handlers.h

@@ -0,0 +1,63 @@
+
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (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.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is libguac.
+ *
+ * The Initial Developer of the Original Code is
+ * Michael Jumper.
+ * Portions created by the Initial Developer are Copyright (C) 2010
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+#ifndef _GUAC_CLIENT_HANDLERS__H
+#define _GUAC_CLIENT_HANDLERS__H
+
+#include "client.h"
+#include "protocol.h"
+
+typedef int __guac_instruction_handler(guac_client* client, guac_instruction* copied);
+
+typedef struct __guac_instruction_handler_mapping {
+
+    char* opcode;
+    __guac_instruction_handler* handler;
+
+} __guac_instruction_handler_mapping;
+
+int __guac_handle_sync(guac_client* client, guac_instruction* instruction);
+int __guac_handle_mouse(guac_client* client, guac_instruction* instruction);
+int __guac_handle_key(guac_client* client, guac_instruction* instruction);
+int __guac_handle_clipboard(guac_client* client, guac_instruction* instruction);
+int __guac_handle_disconnect(guac_client* client, guac_instruction* instruction);
+
+extern __guac_instruction_handler_mapping __guac_instruction_handler_map[];
+
+int __guac_handle_instruction(guac_client* client, guac_instruction* instruction);
+
+#endif

libguac/include/client.h

@@ -42,6 +42,7 @@
 #include <png.h>
 
 #include "guacio.h"
+#include "protocol.h"
 
 /**
  * Provides functions and structures required for defining (and handling) a proxy client.
@@ -104,6 +105,11 @@ typedef int guac_client_clipboard_handler(guac_client* client, char* copied);
  */
 typedef int guac_client_free_handler(void* client);
 
+typedef enum guac_client_state {
+    RUNNING,
+    STOPPING
+} guac_client_state;
+
 /**
  * Guacamole proxy client.
  *
@@ -120,6 +126,10 @@ struct guac_client {
      */
     GUACIO* io;
 
+    guac_client_state state;
+    long last_received_timestamp;
+    long last_sent_timestamp;
+
     /**
      * Reference to dlopen'd client plugin.
      */
@@ -284,4 +294,11 @@ png_byte** guac_alloc_png_buffer(int w, int h, int bpp);
  */
 void guac_free_png_buffer(png_byte** png_buffer, int h);
 
+int guac_client_handle_instruction(guac_client* client, guac_instruction* instruction);
+void guac_client_stop(guac_client* client);
+
+/* FIXME: MOVE THESE TO protocol.h */
+long guac_client_current_timestamp();
+void guac_client_sleep(int millis);
+
 #endif

libguac/src/client-handlers.c

@@ -0,0 +1,98 @@
+
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (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.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is libguac.
+ *
+ * The Initial Developer of the Original Code is
+ * Michael Jumper.
+ * Portions created by the Initial Developer are Copyright (C) 2010
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+#include <stdlib.h>
+
+#include "client.h"
+#include "protocol.h"
+#include "client-handlers.h"
+
+__guac_instruction_handler_mapping __guac_instruction_handler_map[] = {
+   {"sync",       __guac_handle_sync},
+   {"mouse",      __guac_handle_mouse},
+   {"key",        __guac_handle_key},
+   {"clipboard",  __guac_handle_clipboard},
+   {"disconnect", __guac_handle_disconnect},
+   {NULL,         NULL}
+};
+
+
+int __guac_handle_sync(guac_client* client, guac_instruction* instruction) {
+    long timestamp = atol(instruction->argv[0]);
+
+    /* Error if timestamp is in future */
+    if (timestamp > client->last_sent_timestamp)
+        return -1;
+
+    client->last_received_timestamp = timestamp;
+    return 0;
+}
+
+int __guac_handle_mouse(guac_client* client, guac_instruction* instruction) {
+    if (client->mouse_handler)
+        return client->mouse_handler(
+            client,
+            atoi(instruction->argv[0]), /* x */
+            atoi(instruction->argv[1]), /* y */
+            atoi(instruction->argv[2])  /* mask */
+        );
+    return 0;
+}
+
+int __guac_handle_key(guac_client* client, guac_instruction* instruction) {
+    if (client->key_handler)
+        return client->key_handler(
+            client,
+            atoi(instruction->argv[0]), /* keysym */
+            atoi(instruction->argv[1])  /* pressed */
+        );
+    return 0;
+}
+
+int __guac_handle_clipboard(guac_client* client, guac_instruction* instruction) {
+    if (client->clipboard_handler)
+        return client->clipboard_handler(
+            client,
+            guac_unescape_string_inplace(instruction->argv[0]) /* data */
+        );
+    return 0;
+}
+
+int __guac_handle_disconnect(guac_client* client, guac_instruction* instruction) {
+    return -1;
+}
+

libguac/src/client.c

@@ -50,7 +50,7 @@
 #include "guacio.h"
 #include "protocol.h"
 #include "client.h"
-
+#include "client-handlers.h"
 
 png_byte** guac_alloc_png_buffer(int w, int h, int bpp) {
 
@@ -79,6 +79,50 @@ void guac_free_png_buffer(png_byte** png_buffer, int h) {
 
 }
 
+
+long guac_client_current_timestamp() {
+
+#ifdef HAVE_CLOCK_GETTIME
+
+    struct timespec current;
+
+    /* Get current time */
+    clock_gettime(CLOCK_REALTIME, &current);
+    
+    /* Calculate milliseconds */
+    return current.tv_sec * 1000 + current.tv_nsec / 1000000;
+
+#else
+
+    struct timeval current;
+
+    /* Get current time */
+    gettimeofday(&current, NULL);
+    
+    /* Calculate milliseconds */
+    return current.tv_sec * 1000 + current.tv_usec / 1000;
+
+#endif
+
+}
+
+void guac_client_sleep(int millis) {
+
+#ifdef HAVE_NANOSLEEP 
+        struct timespec sleep_period;
+
+        sleep_period.tv_sec = 0;
+        sleep_period.tv_nsec = millis * 1000000L;
+
+        nanosleep(&sleep_period, NULL);
+#elif defined(__MINGW32__)
+        Sleep(millis)
+#else
+#warning No sleep/nanosleep function available. Clients may not perform as expected. Consider patching libguac to add support for your platform.
+#endif
+
+}
+
 guac_client* __guac_alloc_client(GUACIO* io) {
 
     /* Allocate new client (not handoff) */
@@ -87,6 +131,8 @@ guac_client* __guac_alloc_client(GUACIO* io) {
 
     /* Init new client */
     client->io = io;
+    client->last_received_timestamp = client->last_sent_timestamp = guac_client_current_timestamp();
+    client->state = RUNNING;
 
     return client;
 }
@@ -261,6 +307,9 @@ guac_client* guac_get_client(int client_fd) {
 
 }
 
+void guac_client_stop(guac_client* client) {
+    client->state = STOPPING;
+}
 
 void guac_free_client(guac_client* client) {
 
@@ -280,76 +329,19 @@ void guac_free_client(guac_client* client) {
 }
 
 
-long __guac_current_timestamp() {
-
-#ifdef HAVE_CLOCK_GETTIME
-
-    struct timespec current;
-
-    /* Get current time */
-    clock_gettime(CLOCK_REALTIME, &current);
-    
-    /* Calculate milliseconds */
-    return current.tv_sec * 1000 + current.tv_nsec / 1000000;
-
-#else
-
-    struct timeval current;
-
-    /* Get current time */
-    gettimeofday(&current, NULL);
-    
-    /* Calculate milliseconds */
-    return current.tv_sec * 1000 + current.tv_usec / 1000;
-
-#endif
-
-}
-
-
-void __guac_sleep(int millis) {
-
-#ifdef HAVE_NANOSLEEP 
-        struct timespec sleep_period;
-
-        sleep_period.tv_sec = 0;
-        sleep_period.tv_nsec = millis * 1000000L;
-
-        nanosleep(&sleep_period, NULL);
-#elif defined(__MINGW32__)
-        Sleep(millis)
-#else
-#warning No sleep/nanosleep function available. Clients may not perform as expected. Consider patching libguac to add support for your platform.
-#endif
-
-}
-
-
-typedef struct __guac_client_thread_common {
-
-    guac_client* client;
-    long last_received_timestamp;
-    long last_sent_timestamp;
-
-    int client_active;
-
-} __guac_client_thread_common;
-
-
 void* __guac_client_output_thread(void* data) {
 
-    __guac_client_thread_common* common = (__guac_client_thread_common*) data;
+    guac_client* client = (guac_client*) data;
-    guac_client* client = common->client;
     GUACIO* io = client->io;
 
     /* Guacamole client output loop */
-    while (common->client_active) {
+    while (client->state == RUNNING) {
 
         /* Occasionally ping client with sync */
-        long timestamp = __guac_current_timestamp();
+        long timestamp = guac_client_current_timestamp();
-        if (timestamp - common->last_sent_timestamp > GUAC_SYNC_FREQUENCY) {
+        if (timestamp - client->last_sent_timestamp > GUAC_SYNC_FREQUENCY) {
+            client->last_sent_timestamp = timestamp;
             guac_send_sync(io, timestamp);
-            common->last_sent_timestamp = timestamp;
             guac_flush(io);
         }
 
@@ -360,7 +352,7 @@ void* __guac_client_output_thread(void* data) {
             int last_total_written = io->total_written;
 
             /* Only handle messages if synced within threshold */
-            if (common->last_sent_timestamp - common->last_received_timestamp
+            if (client->last_sent_timestamp - client->last_received_timestamp
                     < GUAC_SYNC_THRESHOLD) {
 
                 int retval = client->handle_messages(client);
@@ -373,11 +365,11 @@ void* __guac_client_output_thread(void* data) {
                 if (io->total_written != last_total_written) {
 
                     /* Sleep as necessary */
-                    __guac_sleep(GUAC_SERVER_MESSAGE_HANDLE_FREQUENCY);
+                    guac_client_sleep(GUAC_SERVER_MESSAGE_HANDLE_FREQUENCY);
 
-                    /* Update sync timestamp and send sync instruction */
+                    /* Send sync instruction */
-                    common->last_sent_timestamp = __guac_current_timestamp();
+                    client->last_sent_timestamp = guac_client_current_timestamp();
-                    guac_send_sync(io, common->last_sent_timestamp);
+                    guac_send_sync(io, client->last_sent_timestamp);
 
                 }
 
@@ -388,131 +380,46 @@ void* __guac_client_output_thread(void* data) {
 
         /* If no message handler, just sleep until next sync ping */
         else
-            __guac_sleep(GUAC_SYNC_FREQUENCY);
+            guac_client_sleep(GUAC_SYNC_FREQUENCY);
 
     } /* End of output loop */
 
-    common->client_active = 0;
+    guac_client_stop(client);
     return NULL;
 
 }
 
 void* __guac_client_input_thread(void* data) {
 
-    __guac_client_thread_common* common = (__guac_client_thread_common*) data;
+    guac_client* client = (guac_client*) data;
-    guac_client* client = common->client;
     GUACIO* io = client->io;
 
     guac_instruction instruction;
 
     /* Guacamole client input loop */
-    while (common->client_active) {
+    while (client->state == RUNNING && guac_instructions_waiting(io) > 0) {
-
-        int wait_result = guac_instructions_waiting(io);
-        if (wait_result > 0) {
 
         int retval;
-            retval = guac_read_instruction(io, &instruction); /* 0 if no instructions finished yet, <0 if error or EOF */
+        while ((retval = guac_read_instruction(io, &instruction)) > 0) {
 
-            if (retval > 0) {
+            if (guac_client_handle_instruction(client, &instruction) < 0) {
-           
-                do {
-
-                    if (strcmp(instruction.opcode, "sync") == 0) {
-                        common->last_received_timestamp = atol(instruction.argv[0]);
-                        if (common->last_received_timestamp > common->last_sent_timestamp) {
-                            guac_send_error(io, "Received sync from future.");
                 guac_free_instruction_data(&instruction);
-                            break;
+                guac_client_stop(client);
-                        }
+                return NULL;
-                    }
-                    else if (strcmp(instruction.opcode, "mouse") == 0) {
-                        if (client->mouse_handler)
-                            if (
-                                    client->mouse_handler(
-                                        client,
-                                        atoi(instruction.argv[0]), /* x */
-                                        atoi(instruction.argv[1]), /* y */
-                                        atoi(instruction.argv[2])  /* mask */
-                                    )
-                               ) {
-
-                                GUAC_LOG_ERROR("Error handling mouse instruction");
-                                guac_free_instruction_data(&instruction);
-                                break;
-
-                            }
-                    }
-
-                    else if (strcmp(instruction.opcode, "key") == 0) {
-                        if (client->key_handler)
-                            if (
-                                    client->key_handler(
-                                        client,
-                                        atoi(instruction.argv[0]), /* keysym */
-                                        atoi(instruction.argv[1])  /* pressed */
-                                    )
-                               ) {
-
-                                GUAC_LOG_ERROR("Error handling key instruction");
-                                guac_free_instruction_data(&instruction);
-                                break;
-
-                            }
-                    }
-
-                    else if (strcmp(instruction.opcode, "clipboard") == 0) {
-                        if (client->clipboard_handler)
-                            if (
-                                    client->clipboard_handler(
-                                        client,
-                                        guac_unescape_string_inplace(instruction.argv[0]) /* data */
-                                    )
-                               ) {
-
-                                GUAC_LOG_ERROR("Error handling clipboard instruction");
-                                guac_free_instruction_data(&instruction);
-                                break;
-
-                            }
-                    }
-
-                    else if (strcmp(instruction.opcode, "disconnect") == 0) {
-                        GUAC_LOG_INFO("Client requested disconnect");
-                        guac_free_instruction_data(&instruction);
-                        break;
             }
 
             guac_free_instruction_data(&instruction);
 
-                } while ((retval = guac_read_instruction(io, &instruction)) > 0);
+        }
 
-                if (retval < 0) {
+        if (retval < 0)
-                    GUAC_LOG_ERROR("Error reading instruction from stream");
             break;
-                }
-            }
-
-            if (retval < 0) {
-                GUAC_LOG_ERROR("Error or end of stream");
-                break; /* EOF or error */
-            }
 
         /* Otherwise, retval == 0 implies unfinished instruction */
 
     }
-        else if (wait_result < 0) {
-            GUAC_LOG_ERROR("Error waiting for next instruction");
-            break;
-        }
-        else { /* wait_result == 0 */
-            GUAC_LOG_ERROR("Timeout");
-            break;
-        }
 
-    }
+    guac_client_stop(client);
-
-    common->client_active = 0;
     return NULL;
 
 }
@@ -520,19 +427,13 @@ void* __guac_client_input_thread(void* data) {
 void guac_start_client(guac_client* client) {
 
     pthread_t input_thread, output_thread;
-    __guac_client_thread_common common;
 
-    /* Init thread data */
+    if (pthread_create(&output_thread, NULL, __guac_client_output_thread, (void*) client)) {
-    common.client = client;
-    common.last_received_timestamp = common.last_sent_timestamp = __guac_current_timestamp();
-    common.client_active = 1;
-
-    if (pthread_create(&output_thread, NULL, __guac_client_output_thread, (void*) &common)) {
         /* THIS FUNCTION SHOULD RETURN A VALUE! */
         return;
     }
 
-    if (pthread_create(&input_thread, NULL, __guac_client_input_thread, (void*) &common)) {
+    if (pthread_create(&input_thread, NULL, __guac_client_input_thread, (void*) client)) {
         /* THIS FUNCTION SHOULD RETURN A VALUE! */
         return;
     }
@@ -545,3 +446,21 @@ void guac_start_client(guac_client* client) {
 
 }
 
+int guac_client_handle_instruction(guac_client* client, guac_instruction* instruction) {
+
+    /* For each defined instruction */
+    __guac_instruction_handler_mapping* current = __guac_instruction_handler_map;
+    while (current->opcode != NULL) {
+
+        /* If recognized, call handler */
+        if (strcmp(instruction->opcode, current->opcode) == 0)
+            return current->handler(client, instruction);
+
+        current++;
+    }
+
+    /* If unrecognized, ignore */
+    return 0;
+
+}
+