From: Marc-André Moreau Date: Tue, 12 Feb 2013 01:38:19 +0000 (-0500) Subject: xfreerdp: stabilize asynchronous X11 event handling X-Git-Tag: 1.1.0-beta1~49^2~17 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=47a7d543703a87b3a5a91ee5cc13db6eb7c772b7;p=platform%2Fupstream%2Ffreerdp.git xfreerdp: stabilize asynchronous X11 event handling --- diff --git a/client/X11/xf_cliprdr.c b/client/X11/xf_cliprdr.c index a334c47..3c8f86c 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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);