xfreerdp: stabilize asynchronous X11 event handling

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 */
 

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;

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))

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;
+				}
+			}
 		}
 	}
 

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;
 }
 

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)

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;
 }
 

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
 		}
 	}

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;
 }

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;
-
-	if (queue->size == 0)
-		SetEvent(queue->event);
-
 	queue->size++;
 
+	if (queue->size > 0)
+		SetEvent(queue->event);
+
 	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;
-
-		if (queue->size == 1)
-			ResetEvent(queue->event);
-
 		queue->size--;
 
+		if (queue->size < 1)
+			ResetEvent(queue->event);
+
 		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;
-
-			if (queue->size == 1)
-				ResetEvent(queue->event);
-
 			queue->size--;
+
+			if (queue->size < 1)
+				ResetEvent(queue->event);
 		}
 	}
 

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);