xfreerdp: stabilize asynchronous X11 event handling

diff --git a/client/X11/xf_cliprdr.c b/client/X11/xf_cliprdr.c
index a334c47..3c8f86c 100644 (file)
--- a/client/X11/xf_cliprdr.c
+++ b/client/X11/xf_cliprdr.c
@@ -713,8 +713,7 @@ static BOOL xf_cliprdr_get_requested_data(xfInfo* xfi, Atom target)
        unsigned long length, bytes_left, dummy;
        clipboardContext* cb = (clipboardContext*) xfi->clipboard_context;
 
-       if ((cb->request_index < 0) ||
-               (cb->format_mappings[cb->request_index].target_format != target))
+       if ((cb->request_index < 0) || (cb->format_mappings[cb->request_index].target_format != target))
        {
                DEBUG_X11_CLIPRDR("invalid target");
                xf_cliprdr_send_null_data_response(xfi);
@@ -1132,6 +1131,7 @@ BOOL xf_cliprdr_process_selection_request(xfInfo* xfi, XEvent* xevent)
                {
                        format = cb->format_mappings[i].format_id;
                        alt_format = format;
+
                        if (format == CB_FORMAT_RAW)
                        {
                                if (XGetWindowProperty(xfi->display, xevent->xselectionrequest.requestor,
@@ -1146,7 +1146,9 @@ BOOL xf_cliprdr_process_selection_request(xfInfo* xfi, XEvent* xevent)
                                        XFree(data);
                                }
                        }
+
                        DEBUG_X11_CLIPRDR("provide format 0x%04x alt_format 0x%04x", format, alt_format);
+
                        if ((cb->data != 0) && (format == cb->data_format) && (alt_format == cb->data_alt_format))
                        {
                                /* Cached clipboard data available. Send it now */
@@ -1206,6 +1208,9 @@ BOOL xf_cliprdr_process_property_notify(xfInfo* xfi, XEvent* xevent)
 {
        clipboardContext* cb = (clipboardContext*) xfi->clipboard_context;
 
+       if (!cb)
+               return TRUE;
+
        if (xevent->xproperty.atom != cb->property_atom)
                return FALSE; /* Not cliprdr-related */
 

diff --git a/client/X11/xf_event.c b/client/X11/xf_event.c
index d9f1840..ca4d80f 100644 (file)
--- a/client/X11/xf_event.c
+++ b/client/X11/xf_event.c
@@ -381,7 +381,7 @@ static BOOL xf_event_FocusIn(xfInfo* xfi, XEvent* event, BOOL app)
 
        xfi->focused = TRUE;
 
-       if (xfi->mouse_active && (app != TRUE))
+       if (xfi->mouse_active && (!app))
                XGrabKeyboard(xfi->display, xfi->window->handle, TRUE, GrabModeAsync, GrabModeAsync, CurrentTime);
 
        if (app)
@@ -397,6 +397,7 @@ static BOOL xf_event_FocusIn(xfInfo* xfi, XEvent* event, BOOL app)
                if (window != NULL)
                        xf_rail_adjust_position(xfi, window);
        }
+
        xf_kbd_focus_in(xfi);
 
        if (!app)
@@ -570,20 +571,17 @@ static BOOL xf_event_MapNotify(xfInfo* xfi, XEvent* event, BOOL app)
 {
        RECTANGLE_16 rect;
        rdpWindow* window;
-       rdpUpdate* update = xfi->instance->update;
+       //rdpUpdate* update = xfi->instance->update;
        rdpRail* rail = ((rdpContext*) xfi->context)->rail;
 
        if (!app)
        {
-               if (xfi->suppress_output == TRUE)
-               {
-                       xfi->suppress_output = FALSE;
-                       rect.left = 0;
-                       rect.top = 0;
-                       rect.right = xfi->width;
-                       rect.bottom = xfi->height;
-                       update->SuppressOutput((rdpContext*) xfi->context, 1, &rect);
-               }
+               rect.left = 0;
+               rect.top = 0;
+               rect.right = xfi->width;
+               rect.bottom = xfi->height;
+
+               //update->SuppressOutput((rdpContext*) xfi->context, 1, &rect);
        }
        else
        {
@@ -610,18 +608,14 @@ static BOOL xf_event_MapNotify(xfInfo* xfi, XEvent* event, BOOL app)
 static BOOL xf_event_UnmapNotify(xfInfo* xfi, XEvent* event, BOOL app)
 {
        rdpWindow* window;
-       rdpUpdate* update = xfi->instance->update;
+       //rdpUpdate* update = xfi->instance->update;
        rdpRail* rail = ((rdpContext*) xfi->context)->rail;
 
        xf_kbd_release_all_keypress(xfi);
 
        if (!app)
        {
-               if (xfi->suppress_output == FALSE)
-               {
-                       xfi->suppress_output = TRUE;
-                       update->SuppressOutput((rdpContext*) xfi->context, 0, NULL);
-               }
+               //update->SuppressOutput((rdpContext*) xfi->context, 0, NULL);
        }
        else
        {
@@ -629,7 +623,7 @@ static BOOL xf_event_UnmapNotify(xfInfo* xfi, XEvent* event, BOOL app)
 
                if (window != NULL)
                {
-                       xfWindow *xfw = (xfWindow*) window->extra;
+                       xfWindow* xfw = (xfWindow*) window->extra;
                        xfw->is_mapped = FALSE;
                }
        }
@@ -678,7 +672,7 @@ static BOOL xf_event_PropertyNotify(xfInfo* xfi, XEvent* event, BOOL app)
         * ie. not using the buttons on the rail window itself
         */
 
-       if (app == TRUE)
+       if (app)
        {
                rdpWindow* window;
                
@@ -761,9 +755,7 @@ static BOOL xf_event_PropertyNotify(xfInfo* xfi, XEvent* event, BOOL app)
                          }
                }       
         }
-       
-
-       if (!app)
+       else
        {
                if (xf_cliprdr_process_property_notify(xfi, event))
                        return TRUE;
@@ -867,7 +859,7 @@ BOOL xf_event_process(freerdp* instance, XEvent* event)
                if (window) 
                {
                        /* Update "current" window for cursor change orders */
-                       xfi->window = (xfWindow *) window->extra;
+                       xfi->window = (xfWindow*) window->extra;
 
                        if (xf_event_suppress_events(xfi, window, event))
                                return TRUE;

diff --git a/client/X11/xf_keyboard.c b/client/X11/xf_keyboard.c
index 3d38c80..3640ed5 100644 (file)
--- a/client/X11/xf_keyboard.c
+++ b/client/X11/xf_keyboard.c
@@ -41,6 +41,7 @@ void xf_kbd_init(xfInfo* xfi)
        xfi->keyboard_layout_id = xfi->instance->settings->KeyboardLayout;
        xfi->keyboard_layout_id = freerdp_keyboard_init(xfi->keyboard_layout_id);
        xfi->instance->settings->KeyboardLayout = xfi->keyboard_layout_id;
+       xfi->modifier_map = XGetModifierMapping(xfi->display);
 }
 
 void xf_kbd_clear(xfInfo* xfi)
@@ -173,6 +174,7 @@ int xf_kbd_get_toggle_keys_state(xfInfo* xfi)
        int toggle_keys_state = 0;
 
        state = xf_kbd_read_keyboard_state(xfi);
+
        if (xf_kbd_get_key_state(xfi, state, XK_Scroll_Lock))
                toggle_keys_state |= KBD_SYNC_SCROLL_LOCK;
        if (xf_kbd_get_key_state(xfi, state, XK_Num_Lock))

diff --git a/client/X11/xfreerdp.c b/client/X11/xfreerdp.c
index 72a946f..f5804d6 100644 (file)
--- a/client/X11/xfreerdp.c
+++ b/client/X11/xfreerdp.c
@@ -588,6 +588,8 @@ BOOL xf_pre_connect(freerdp* instance)
 
        ((xfContext*) instance->context)->xfi = xfi;
 
+       xfi->mutex = CreateMutex(NULL, FALSE, NULL);
+
        xfi->_context = instance->context;
        xfi->context = (xfContext*) instance->context;
        xfi->context->settings = instance->settings;
@@ -1117,19 +1119,26 @@ void xf_free(xfInfo* xfi)
 
 void* xf_update_thread(void* arg)
 {
+       int status;
        wMessage message;
        wMessageQueue* queue;
        freerdp* instance = (freerdp*) arg;
 
+       status = 1;
        queue = freerdp_get_message_queue(instance, FREERDP_UPDATE_MESSAGE_QUEUE);
 
        while (MessageQueue_Wait(queue))
        {
                while (MessageQueue_Peek(queue, &message, TRUE))
                {
-                       if (!freerdp_message_queue_process_message(instance, FREERDP_UPDATE_MESSAGE_QUEUE, &message))
+                       status = freerdp_message_queue_process_message(instance, FREERDP_UPDATE_MESSAGE_QUEUE, &message);
+
+                       if (!status)
                                break;
                }
+
+               if (!status)
+                       break;
        }
 
        return NULL;
@@ -1140,6 +1149,7 @@ void* xf_input_thread(void* arg)
        xfInfo* xfi;
        HANDLE event;
        XEvent xevent;
+       wMessageQueue* queue;
        int pending_status = 1;
        int process_status = 1;
        freerdp* instance = (freerdp*) arg;
@@ -1178,8 +1188,8 @@ void* xf_input_thread(void* arg)
                        break;
        }
 
-       xfi->disconnect = TRUE;
-       printf("Closed from X\n");
+       queue = freerdp_get_message_queue(instance, FREERDP_INPUT_MESSAGE_QUEUE);
+       MessageQueue_PostQuit(queue, 0);
 
        return NULL;
 }
@@ -1246,6 +1256,8 @@ int xfreerdp_run(freerdp* instance)
 
        status = freerdp_connect(instance);
 
+       xfi = ((xfContext*) instance->context)->xfi;
+
        /* Connection succeeded. --authonly ? */
        if (instance->settings->AuthenticationOnly)
        {
@@ -1256,19 +1268,16 @@ int xfreerdp_run(freerdp* instance)
 
        if (!status)
        {
-               xf_free(((xfContext*) instance->context)->xfi);
+               xf_free(xfi);
                return XF_EXIT_CONN_FAILED;
        }
 
-       xfi = ((xfContext*) instance->context)->xfi;
        channels = instance->context->channels;
        settings = instance->context->settings;
 
        async_update = settings->AsyncUpdate;
        async_input = settings->AsyncInput;
 
-       xfi->mutex = CreateMutex(NULL, FALSE, NULL);
-
        if (async_update)
        {
                update_thread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE) xf_update_thread, instance, 0, NULL);
@@ -1276,12 +1285,6 @@ int xfreerdp_run(freerdp* instance)
 
        if (async_input)
        {
-               /**
-                * FIXME: the input event file descriptor doesn't seem to work well with select(), why?
-                */
-               input_event = freerdp_get_message_queue_event_handle(instance, FREERDP_INPUT_MESSAGE_QUEUE);
-               fd_input_event = GetEventFileDescriptor(input_event);
-
                input_thread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE) xf_input_thread, instance, 0, NULL);
        }
 
@@ -1314,19 +1317,21 @@ int xfreerdp_run(freerdp* instance)
                        }
                }
 
-               //if (!async_input)
-               //{
+               if (!async_input)
+               {
                        if (xf_get_fds(instance, rfds, &rcount, wfds, &wcount) != TRUE)
                        {
                                printf("Failed to get xfreerdp file descriptor\n");
                                ret = XF_EXIT_CONN_FAILED;
                                break;
                        }
-               //}
-               //else
-               //{
-               //      rfds[rcount++] = (void*) (long) fd_input_event;
-               //}
+               }
+               else
+               {
+                       input_event = freerdp_get_message_queue_event_handle(instance, FREERDP_INPUT_MESSAGE_QUEUE);
+                       fd_input_event = GetEventFileDescriptor(input_event);
+                       rfds[rcount++] = (void*) (long) fd_input_event;
+               }
 
                max_fds = 0;
                FD_ZERO(&rfds_set);
@@ -1345,10 +1350,10 @@ int xfreerdp_run(freerdp* instance)
                if (max_fds == 0)
                        break;
 
-               timeout.tv_sec = 0;
-               timeout.tv_usec = 100;
+               timeout.tv_sec = 1;
+               timeout.tv_usec = 0;
 
-               select_status = select(max_fds + 1, &rfds_set, &wfds_set, NULL, NULL);
+               select_status = select(max_fds + 1, &rfds_set, NULL, NULL, &timeout);
 
                if (select_status == 0)
                {
@@ -1386,14 +1391,21 @@ int xfreerdp_run(freerdp* instance)
                {
                        if (xf_process_x_events(instance) != TRUE)
                        {
-                               printf("Closed from X\n");
+                               printf("Closed from X11\n");
                                break;
                        }
                }
                else
                {
-                       //if (WaitForSingleObject(input_event, 0) == WAIT_OBJECT_0)
-                               freerdp_message_queue_process_pending_messages(instance, FREERDP_INPUT_MESSAGE_QUEUE);
+                       if (WaitForSingleObject(input_event, 0) == WAIT_OBJECT_0)
+                       {
+                               if (!freerdp_message_queue_process_pending_messages(instance, FREERDP_INPUT_MESSAGE_QUEUE))
+                               {
+                                       printf("User Disconnect\n");
+                                       xfi->disconnect = TRUE;
+                                       break;
+                               }
+                       }
                }
        }
 

diff --git a/libfreerdp/core/capabilities.c b/libfreerdp/core/capabilities.c
index 34bc460..ef99a36 100644 (file)
--- a/libfreerdp/core/capabilities.c
+++ b/libfreerdp/core/capabilities.c
@@ -123,8 +123,9 @@ BOOL rdp_read_general_capability_set(STREAM* s, UINT16 length, rdpSettings* sett
        BYTE refreshRectSupport;
        BYTE suppressOutputSupport;
 
-       if(length < 24)
+       if (length < 24)
                return FALSE;
+
        if (settings->ServerMode)
        {
                stream_read_UINT16(s, settings->OsMajorType); /* osMajorType (2 bytes) */
@@ -135,6 +136,7 @@ BOOL rdp_read_general_capability_set(STREAM* s, UINT16 length, rdpSettings* sett
                stream_seek_UINT16(s); /* osMajorType (2 bytes) */
                stream_seek_UINT16(s); /* osMinorType (2 bytes) */
        }
+
        stream_seek_UINT16(s); /* protocolVersion (2 bytes) */
        stream_seek_UINT16(s); /* pad2OctetsA (2 bytes) */
        stream_seek_UINT16(s); /* generalCompressionTypes (2 bytes) */
@@ -153,6 +155,7 @@ BOOL rdp_read_general_capability_set(STREAM* s, UINT16 length, rdpSettings* sett
 
        if (suppressOutputSupport == FALSE)
                settings->SuppressOutput = FALSE;
+
        return TRUE;
 }
 

diff --git a/libfreerdp/core/message.c b/libfreerdp/core/message.c
index 714e9c6..d484c69 100644 (file)
--- a/libfreerdp/core/message.c
+++ b/libfreerdp/core/message.c
@@ -127,10 +127,13 @@ static void update_message_RefreshRect(rdpContext* context, BYTE count, RECTANGL
 
 static void update_message_SuppressOutput(rdpContext* context, BYTE allow, RECTANGLE_16* area)
 {
-       RECTANGLE_16* lParam;
+       RECTANGLE_16* lParam = NULL;
 
-       lParam = (RECTANGLE_16*) malloc(sizeof(RECTANGLE_16));
-       CopyMemory(lParam, area, sizeof(RECTANGLE_16));
+       if (area)
+       {
+               lParam = (RECTANGLE_16*) malloc(sizeof(RECTANGLE_16));
+               CopyMemory(lParam, area, sizeof(RECTANGLE_16));
+       }
 
        MessageQueue_Post(context->update->queue, (void*) context,
                        MakeMessageId(Update, SuppressOutput), (void*) (size_t) allow, (void*) lParam);
@@ -974,7 +977,8 @@ int update_message_process_update_class(rdpUpdateProxy* proxy, wMessage* msg, in
                case Update_SuppressOutput:
                        IFCALL(proxy->SuppressOutput, msg->context,
                                        (BYTE) (size_t) msg->wParam, (RECTANGLE_16*) msg->lParam);
-                       free(msg->lParam);
+                       if (msg->lParam)
+                               free(msg->lParam);
                        break;
 
                case Update_SurfaceCommand:
@@ -1497,7 +1501,7 @@ int update_message_queue_process_message(rdpUpdate* update, wMessage* message)
        status = update_message_process_class(update->proxy, message, msgClass, msgType);
 
        if (status < 0)
-               return -1;
+               status = -1;
 
        return 1;
 }
@@ -1508,15 +1512,11 @@ int update_message_queue_process_pending_messages(rdpUpdate* update)
        wMessage message;
        wMessageQueue* queue;
 
+       status = 1;
        queue = update->queue;
 
-       while (1)
+       while (MessageQueue_Peek(queue, &message, TRUE))
        {
-               status = MessageQueue_Peek(queue, &message, TRUE);
-
-               if (!status)
-                       break;
-
                status = update_message_queue_process_message(update, &message);
 
                if (!status)
@@ -1871,6 +1871,7 @@ int input_message_queue_process_pending_messages(rdpInput* input)
        wMessageQueue* queue;
 
        count = 0;
+       status = 1;
        queue = input->queue;
 
        while (MessageQueue_Peek(queue, &message, TRUE))
@@ -1883,7 +1884,7 @@ int input_message_queue_process_pending_messages(rdpInput* input)
                count++;
        }
 
-       return 0;
+       return status;
 }
 
 void input_message_proxy_register(rdpInputProxy* proxy, rdpInput* input)

diff --git a/libfreerdp/core/rdp.c b/libfreerdp/core/rdp.c
index 3251883..e9ceeae 100644 (file)
--- a/libfreerdp/core/rdp.c
+++ b/libfreerdp/core/rdp.c
@@ -483,12 +483,14 @@ BOOL rdp_send_data_pdu(rdpRdp* rdp, STREAM* s, BYTE type, UINT16 channel_id)
 
 BOOL rdp_recv_set_error_info_data_pdu(rdpRdp* rdp, STREAM* s)
 {
-       if(stream_get_left(s) < 4)
+       if (stream_get_left(s) < 4)
                return FALSE;
+
        stream_read_UINT32(s, rdp->errorInfo); /* errorInfo (4 bytes) */
 
        if (rdp->errorInfo != ERRINFO_SUCCESS)
                rdp_print_errinfo(rdp->errorInfo);
+
        return TRUE;
 }
 

diff --git a/winpr/libwinpr/synch/event.c b/winpr/libwinpr/synch/event.c
index 7fb04b8..4b49a4d 100644 (file)
--- a/winpr/libwinpr/synch/event.c
+++ b/winpr/libwinpr/synch/event.c
@@ -185,15 +185,13 @@ BOOL ResetEvent(HANDLE hEvent)
                        }
                        while ((length < 0) && (errno == EINTR));
 
-                       status = (length > 0) ? TRUE : FALSE;
+                       if ((length > 0) && (!status))
+                               status = TRUE;
 #else
                        length = read(event->pipe_fd[0], &length, 1);
 
-                       if (length == 1)
+                       if ((length == 1) && (!status))
                                status = TRUE;
-
-                       if (length != 1)
-                               break;
 #endif
                }
        }

diff --git a/winpr/libwinpr/synch/wait.c b/winpr/libwinpr/synch/wait.c
index e2b7ac3..94387e5 100644 (file)
--- a/winpr/libwinpr/synch/wait.c
+++ b/winpr/libwinpr/synch/wait.c
@@ -65,7 +65,7 @@ DWORD WaitForSingleObject(HANDLE hHandle, DWORD dwMilliseconds)
                if (status != 0)
                        printf("WaitForSingleObject: pthread_join failure: %d\n", status);
        }
-       if (Type == HANDLE_TYPE_MUTEX)
+       else if (Type == HANDLE_TYPE_MUTEX)
        {
                if (dwMilliseconds != INFINITE)
                        printf("WaitForSingleObject: timeout not implemented for mutex wait\n");
@@ -90,7 +90,7 @@ DWORD WaitForSingleObject(HANDLE hHandle, DWORD dwMilliseconds)
                        timeout.tv_usec = dwMilliseconds * 1000;
                }
 
-               status = select(event->pipe_fd[0] + 1, &rfds, 0, 0,
+               status = select(event->pipe_fd[0] + 1, &rfds, NULL, NULL,
                                (dwMilliseconds == INFINITE) ? NULL : &timeout);
 
                if (status < 0)
@@ -146,6 +146,10 @@ DWORD WaitForSingleObject(HANDLE hHandle, DWORD dwMilliseconds)
 
 #endif
        }
+       else
+       {
+               printf("WaitForSingleObject: unknown handle type %d\n", Type);
+       }
 
        return WAIT_OBJECT_0;
 }

diff --git a/winpr/libwinpr/utils/collections/MessageQueue.c b/winpr/libwinpr/utils/collections/MessageQueue.c
index fc8da26..a2b7e37 100644 (file)
--- a/winpr/libwinpr/utils/collections/MessageQueue.c
+++ b/winpr/libwinpr/utils/collections/MessageQueue.c
@@ -91,12 +91,11 @@ void MessageQueue_Dispatch(wMessageQueue* queue, wMessage* message)
 
        CopyMemory(&(queue->array[queue->tail]), message, sizeof(wMessage));
        queue->tail = (queue->tail + 1) % queue->capacity;
+       queue->size++;
 
-       if (queue->size == 0)
+       if (queue->size > 0)
                SetEvent(queue->event);
 
-       queue->size++;
-
        ReleaseMutex(queue->mutex);
 }
 
@@ -131,12 +130,11 @@ int MessageQueue_Get(wMessageQueue* queue, wMessage* message)
                CopyMemory(message, &(queue->array[queue->head]), sizeof(wMessage));
                ZeroMemory(&(queue->array[queue->head]), sizeof(wMessage));
                queue->head = (queue->head + 1) % queue->capacity;
+               queue->size--;
 
-               if (queue->size == 1)
+               if (queue->size < 1)
                        ResetEvent(queue->event);
 
-               queue->size--;
-
                status = (message->id != WMQ_QUIT) ? 1 : 0;
        }
 
@@ -160,11 +158,10 @@ int MessageQueue_Peek(wMessageQueue* queue, wMessage* message, BOOL remove)
                {
                        ZeroMemory(&(queue->array[queue->head]), sizeof(wMessage));
                        queue->head = (queue->head + 1) % queue->capacity;
+                       queue->size--;
 
-                       if (queue->size == 1)
+                       if (queue->size < 1)
                                ResetEvent(queue->event);
-
-                       queue->size--;
                }
        }
 

diff --git a/winpr/libwinpr/utils/test/TestMessageQueue.c b/winpr/libwinpr/utils/test/TestMessageQueue.c
index 56232ba..2185cab 100644 (file)
--- a/winpr/libwinpr/utils/test/TestMessageQueue.c
+++ b/winpr/libwinpr/utils/test/TestMessageQueue.c
@@ -29,8 +29,6 @@ int TestMessageQueue(int argc, char* argv[])
        HANDLE thread;
        wMessageQueue* queue;
 
-       printf("Message Queue\n");
-
        queue = MessageQueue_New();
 
        thread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE) message_queue_consumer_thread, (void*) queue, 0, NULL);