From 3b54ba3449e933e021185ef6a080e15a282949a6 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 13:38:35 +0200 Subject: [PATCH 01/19] Fixed thread handling. --- channels/audin/server/audin.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/channels/audin/server/audin.c b/channels/audin/server/audin.c index 564630d8c..b597150fa 100644 --- a/channels/audin/server/audin.c +++ b/channels/audin/server/audin.c @@ -419,7 +419,9 @@ static BOOL audin_server_close(audin_server_context* context) SetEvent(audin->stopEvent); WaitForSingleObject(audin->thread, INFINITE); CloseHandle(audin->thread); + CloseHandle(audin->stopEvent); audin->thread = NULL; + audin->stopEvent = NULL; } if (audin->audin_channel) From 1d416ff387ec0eedc9a8ee57ed97e072140d0601 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 13:38:47 +0200 Subject: [PATCH 02/19] Fixed thread handling. --- channels/drive/client/drive_main.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/channels/drive/client/drive_main.c b/channels/drive/client/drive_main.c index d4baee68c..809123290 100644 --- a/channels/drive/client/drive_main.c +++ b/channels/drive/client/drive_main.c @@ -633,17 +633,18 @@ static void drive_process_irp_list(DRIVE_DEVICE* disk) static void* drive_thread_func(void* arg) { DRIVE_DEVICE* disk = (DRIVE_DEVICE*) arg; + HANDLE hdl[] = {disk->irpEvent, disk->stopEvent}; while (1) { - WaitForSingleObject(disk->irpEvent, INFINITE); - - if (WaitForSingleObject(disk->stopEvent, 0) == WAIT_OBJECT_0) + DWORD rc = WaitForMultipleObjects(2, hdl, FALSE, INFINITE); + if (rc == WAIT_OBJECT_0 + 1) break; ResetEvent(disk->irpEvent); drive_process_irp_list(disk); } + ExitThread(0); return NULL; } @@ -664,8 +665,10 @@ static void drive_free(DEVICE* device) DRIVE_DEVICE* disk = (DRIVE_DEVICE*) device; SetEvent(disk->stopEvent); + WaitForSingleObject(disk->thread, INFINITE); CloseHandle(disk->thread); CloseHandle(disk->irpEvent); + CloseHandle(disk->stopEvent); while ((irp = (IRP*) InterlockedPopEntrySList(disk->pIrpList)) != NULL) irp->Discard(irp); @@ -727,7 +730,7 @@ void drive_register_drive_path(PDEVICE_SERVICE_ENTRY_POINTS pEntryPoints, char* pEntryPoints->RegisterDevice(pEntryPoints->devman, (DEVICE*) disk); - ResumeThread(disk->thread); + ResumeThread(disk->thread); } } From d2fec8f612735943e2df6278d754bb3b7072d217 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 13:38:56 +0200 Subject: [PATCH 03/19] Fixed thread handling. --- channels/printer/client/printer_main.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/channels/printer/client/printer_main.c b/channels/printer/client/printer_main.c index 3826b7c3a..b3b17fb04 100644 --- a/channels/printer/client/printer_main.c +++ b/channels/printer/client/printer_main.c @@ -200,6 +200,8 @@ static void* printer_thread_func(void* arg) printer_process_irp(printer_dev, irp); } + ExitThread(0); + return NULL; } @@ -219,11 +221,14 @@ static void printer_free(DEVICE* device) SetEvent(printer_dev->stopEvent); WaitForSingleObject(printer_dev->thread, INFINITE); - CloseHandle(printer_dev->thread); while ((irp = (IRP*) InterlockedPopEntrySList(printer_dev->pIrpList)) != NULL) irp->Discard(irp); + CloseHandle(printer_dev->thread); + CloseHandle(printer_dev->stopEvent); + CloseHandle(printer_dev->event); + _aligned_free(printer_dev->pIrpList); if (printer_dev->printer) From 733a39779c075672498f8b0cf77cb7c490e0cd42 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 13:39:05 +0200 Subject: [PATCH 04/19] Fixed thread handling. --- channels/rdpei/client/rdpei_main.c | 41 ++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/channels/rdpei/client/rdpei_main.c b/channels/rdpei/client/rdpei_main.c index 81099e6d8..b332cec5e 100644 --- a/channels/rdpei/client/rdpei_main.c +++ b/channels/rdpei/client/rdpei_main.c @@ -21,6 +21,7 @@ #include "config.h" #endif +#include #include #include #include @@ -97,6 +98,7 @@ struct _RDPEI_PLUGIN RDPINPUT_CONTACT_POINT* contactPoints; HANDLE event; + HANDLE stopEvent; HANDLE thread; CRITICAL_SECTION lock; @@ -156,10 +158,16 @@ static void* rdpei_schedule_thread(void* arg) DWORD status; RDPEI_PLUGIN* rdpei = (RDPEI_PLUGIN*) arg; RdpeiClientContext* context = (RdpeiClientContext*) rdpei->iface.pInterface; + HANDLE hdl[] = {rdpei->event, rdpei->stopEvent}; + + assert(NULL != rdpei); + assert(NULL != context); while (1) { - status = WaitForSingleObject(rdpei->event, 20); + status = WaitForMultipleObjects(2, hdl, FALSE, 20); + if (status == WAIT_OBJECT_0 + 1) + break; EnterCriticalSection(&rdpei->lock); @@ -174,6 +182,8 @@ static void* rdpei_schedule_thread(void* arg) LeaveCriticalSection(&rdpei->lock); } + ExitThread(0); + return NULL; } @@ -218,13 +228,6 @@ int rdpei_send_cs_ready_pdu(RDPEI_CHANNEL_CALLBACK* callback) Stream_SealLength(s); - if (!rdpei->thread) - { - InitializeCriticalSection(&rdpei->lock); - rdpei->event = CreateEvent(NULL, TRUE, FALSE, NULL); - rdpei->thread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE) rdpei_schedule_thread, (void*) rdpei, 0, NULL); - } - status = rdpei_send_pdu(callback, s, EVENTID_CS_READY, pduLength); Stream_Free(s, TRUE); @@ -487,6 +490,12 @@ static int rdpei_plugin_initialize(IWTSPlugin* pPlugin, IWTSVirtualChannelManage (IWTSListenerCallback*) rdpei->listener_callback, &(rdpei->listener)); rdpei->listener->pInterface = rdpei->iface.pInterface; + + InitializeCriticalSection(&rdpei->lock); + rdpei->event = CreateEvent(NULL, TRUE, FALSE, NULL); + rdpei->stopEvent = CreateEvent(NULL, TRUE, FALSE, NULL); + rdpei->thread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE) + rdpei_schedule_thread, (void*) rdpei, 0, NULL); return status; } @@ -497,6 +506,22 @@ static int rdpei_plugin_terminated(IWTSPlugin* pPlugin) DEBUG_DVC(""); + assert(NULL != pPlugin); + + SetEvent(rdpei->stopEvent); + EnterCriticalSection(&rdpei->lock); + + WaitForSingleObject(rdpei->thread, INFINITE); + + CloseHandle(rdpei->stopEvent); + CloseHandle(rdpei->event); + CloseHandle(rdpei->thread); + + DeleteCriticalSection(&rdpei->lock); + + if (rdpei->listener_callback) + free(rdpei->listener_callback); + free(rdpei); return 0; From d7d5869cb60d8bb26f751b4a5d5dea0196f2037f Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 13:39:15 +0200 Subject: [PATCH 05/19] Fixed thread handling. --- channels/rdpsnd/server/rdpsnd.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/channels/rdpsnd/server/rdpsnd.c b/channels/rdpsnd/server/rdpsnd.c index 57a9593a2..2735156c9 100644 --- a/channels/rdpsnd/server/rdpsnd.c +++ b/channels/rdpsnd/server/rdpsnd.c @@ -541,6 +541,9 @@ void rdpsnd_server_context_free(rdpsnd_server_context* context) SetEvent(rdpsnd->StopEvent); WaitForSingleObject(rdpsnd->thread, INFINITE); + CloseHandle(rdpsnd->StopEvent); + CloseHandle(rdpsnd->thread); + if (rdpsnd->rdpsnd_channel) WTSVirtualChannelClose(rdpsnd->rdpsnd_channel); From e828cef9813dc5be125bfa4692d2bd2f5a8b74f9 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 13:39:24 +0200 Subject: [PATCH 06/19] Fixed thread handling. --- server/X11/xf_peer.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/server/X11/xf_peer.c b/server/X11/xf_peer.c index 7b08eff1a..de6397398 100644 --- a/server/X11/xf_peer.c +++ b/server/X11/xf_peer.c @@ -21,6 +21,7 @@ #include "config.h" #endif +#include #include #include #include @@ -175,6 +176,17 @@ int xf_xshm_init(xfInfo* xfi) return 0; } +void xf_info_free(xfInfo *info) +{ + assert(NULL != info); + + if (info->display) + XCloseDisplay(info->display); + + freerdp_clrconv_free(info->clrconv); + free(info); +} + xfInfo* xf_info_init() { int i; @@ -313,6 +325,11 @@ void xf_peer_context_free(freerdp_peer* client, xfPeerContext* context) { if (context) { + xf_info_free(context->info); + + CloseHandle(context->updateReadyEvent); + CloseHandle(context->updateSentEvent); + Stream_Free(context->s, TRUE); rfx_context_free(context->rfx_context); } @@ -508,6 +525,8 @@ static void* xf_peer_main_loop(void* arg) struct timeval timeout; freerdp_peer* client = (freerdp_peer*) arg; + assert(NULL != client); + ZeroMemory(rfds, sizeof(rfds)); ZeroMemory(&timeout, sizeof(struct timeval)); @@ -598,6 +617,8 @@ static void* xf_peer_main_loop(void* arg) freerdp_peer_context_free(client); freerdp_peer_free(client); + ExitThread(0); + return NULL; } From 8ae13cdace7c58b23bdfc8ebabebe64f3a207b20 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 13:39:32 +0200 Subject: [PATCH 07/19] Fixed thread hanling. --- libfreerdp/utils/svc_plugin.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libfreerdp/utils/svc_plugin.c b/libfreerdp/utils/svc_plugin.c index f0869a6c6..e07e8b353 100644 --- a/libfreerdp/utils/svc_plugin.c +++ b/libfreerdp/utils/svc_plugin.c @@ -185,6 +185,8 @@ static void* svc_plugin_thread_func(void* arg) DEBUG_SVC("in"); + assert(NULL != plugin); + IFCALL(plugin->connect_callback, plugin); while (1) @@ -212,6 +214,8 @@ static void* svc_plugin_thread_func(void* arg) DEBUG_SVC("out"); + ExitThread(0); + return 0; } From c5a632d8f9b263560c0f837a2ba2c4f61826d20f Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 13:39:43 +0200 Subject: [PATCH 08/19] Fixed thread handling. --- client/Windows/wf_interface.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/client/Windows/wf_interface.c b/client/Windows/wf_interface.c index 9ae2de7df..b69c29794 100644 --- a/client/Windows/wf_interface.c +++ b/client/Windows/wf_interface.c @@ -569,7 +569,10 @@ DWORD WINAPI wf_client_thread(LPVOID lpParam) rdpChannels* channels; instance = (freerdp*) lpParam; + assert(NULL != instance); + wfc = (wfContext*) instance->context; + assert(NULL != wfc); ZeroMemory(rfds, sizeof(rfds)); ZeroMemory(wfds, sizeof(wfds)); @@ -690,13 +693,13 @@ DWORD WINAPI wf_client_thread(LPVOID lpParam) } /* cleanup */ - wfc->mainThreadId = 0; - freerdp_channels_close(channels, instance); freerdp_disconnect(instance); printf("Main thread exited.\n"); + ExitThread(0); + return 0; } @@ -708,6 +711,7 @@ DWORD WINAPI wf_keyboard_thread(LPVOID lpParam) HHOOK hook_handle; wfc = (wfContext*) lpParam; + assert(NULL != wfc); hook_handle = SetWindowsHookEx(WH_KEYBOARD_LL, wf_ll_kbd_proc, wfc->hInstance, 0); @@ -734,9 +738,9 @@ DWORD WINAPI wf_keyboard_thread(LPVOID lpParam) fprintf(stderr, "failed to install keyboard hook\n"); } - wfc->keyboardThreadId = 0; printf("Keyboard thread exited.\n"); + ExitThread(0); return (DWORD) NULL; } @@ -1078,12 +1082,27 @@ int wfreerdp_client_stop(rdpContext* context) { wfContext* wfc = (wfContext*) context; - if (wfc->mainThreadId) + if (wfc->thread) + { PostThreadMessage(wfc->mainThreadId, WM_QUIT, 0, 0); - if (wfc->keyboardThreadId) + WaitForSingleObject(wfc->thread, INFINITE); + CloseHandle(wfc->thread); + wfc->thread = NULL; + wfc->mainThreadId = 0; + } + + if (wfc->keyboardThread) + { PostThreadMessage(wfc->keyboardThreadId, WM_QUIT, 0, 0); + WaitForSingleObject(wfc->keyboardThread, INFINITE); + CloseHandle(wfc->keyboardThread); + + wfc->keyboardThread = NULL; + wfc->keyboardThreadId = 0; + } + return 0; } From 2283a5ccdf8430e26af5e1f28a3f55de1b061bc2 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 13:39:54 +0200 Subject: [PATCH 09/19] Fixed thread handling. --- server/X11/xf_interface.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/server/X11/xf_interface.c b/server/X11/xf_interface.c index 1c93b5e4e..b4e95fd7a 100644 --- a/server/X11/xf_interface.c +++ b/server/X11/xf_interface.c @@ -17,6 +17,7 @@ * limitations under the License. */ +#include #include #include #include @@ -93,7 +94,7 @@ void* xf_server_thread(void* param) } } - listener->Close(listener); + ExitThread(0); return NULL; } @@ -115,9 +116,13 @@ int freerdp_server_global_uninit() int freerdp_server_start(xfServer* server) { + assert(NULL != server); + + server->thread = NULL; if (server->listener->Open(server->listener, NULL, 3389)) { - server->thread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE) xf_server_thread, (void*) server, 0, NULL); + server->thread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE) + xf_server_thread, (void*) server, 0, NULL); } return 0; @@ -125,6 +130,17 @@ int freerdp_server_start(xfServer* server) int freerdp_server_stop(xfServer* server) { + if (server->thread) + { + /* ATTENTION: Terminate thread kills a thread, assure + * no resources are allocated during thread execution, + * as they will not be freed! */ + TerminateThread(server->thread, 0); + WaitForSingleObject(server->thread, INFINITE); + CloseHandle(server->thread); + + server->listener->Close(server->listener); + } return 0; } From 777d8c353506b3b021b5ed48813b4267063caf61 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 13:40:04 +0200 Subject: [PATCH 10/19] Fixed thread handling. --- libfreerdp/core/transport.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/libfreerdp/core/transport.c b/libfreerdp/core/transport.c index 9aa30fd71..17c7be411 100644 --- a/libfreerdp/core/transport.c +++ b/libfreerdp/core/transport.c @@ -85,6 +85,14 @@ BOOL transport_disconnect(rdpTransport* transport) status &= tcp_disconnect(transport->TcpIn); } + if (transport->async) + { + SetEvent(transport->stopEvent); + WaitForSingleObject(transport->thread, INFINITE); + + CloseHandle(transport->thread); + CloseHandle(transport->stopEvent); + } return status; } @@ -793,8 +801,14 @@ static void* transport_client_thread(void* arg) rdpTransport* transport; transport = (rdpTransport*) arg; + assert(NULL != transport); + assert(NULL != transport->settings); + instance = (freerdp*) transport->settings->instance; + assert(NULL != instance); + context = instance->context; + assert(NULL != instance->context); while (1) { @@ -802,26 +816,21 @@ static void* transport_client_thread(void* arg) events[nCount++] = transport->stopEvent; events[nCount] = transport->connectedEvent; - status = WaitForMultipleObjects(nCount + 1, events, FALSE, INFINITE); - - if (WaitForSingleObject(transport->stopEvent, 0) == WAIT_OBJECT_0) - { - break; - } - - transport_get_read_handles(transport, (HANDLE*) &events, &nCount); - status = WaitForMultipleObjects(nCount, events, FALSE, INFINITE); - - if (WaitForSingleObject(transport->stopEvent, 0) == WAIT_OBJECT_0) - { + if (status == WAIT_OBJECT_0) + break; + + transport_get_read_handles(transport, (HANDLE*) events, &nCount); + status = WaitForMultipleObjects(nCount, events, FALSE, INFINITE); + if (status == WAIT_OBJECT_0) break; - } if (!freerdp_check_fds(instance)) break; } + ExitThread(0); + return NULL; } From 6c7ad722952683310b19700ddc92adf27599361d Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 13:40:16 +0200 Subject: [PATCH 11/19] Fixed thread handling. --- channels/smartcard/client/smartcard_main.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/channels/smartcard/client/smartcard_main.c b/channels/smartcard/client/smartcard_main.c index 855df3ff0..90eed80aa 100644 --- a/channels/smartcard/client/smartcard_main.c +++ b/channels/smartcard/client/smartcard_main.c @@ -112,13 +112,16 @@ struct _SMARTCARD_IRP_WORKER }; typedef struct _SMARTCARD_IRP_WORKER SMARTCARD_IRP_WORKER; -static void smartcard_process_irp_thread_func(SMARTCARD_IRP_WORKER* irpWorker) +static void *smartcard_process_irp_thread_func(SMARTCARD_IRP_WORKER* irpWorker) { smartcard_process_irp(irpWorker->smartcard, irpWorker->irp); CloseHandle(irpWorker->thread); free(irpWorker); + + ExitThread(0); + return NULL; } static void* smartcard_thread_func(void* arg) From 214807731fc265083e4f4b80fd9ae846379babc2 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 13:44:12 +0200 Subject: [PATCH 12/19] Added missing assert include. --- libfreerdp/core/transport.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libfreerdp/core/transport.c b/libfreerdp/core/transport.c index 17c7be411..933ad428b 100644 --- a/libfreerdp/core/transport.c +++ b/libfreerdp/core/transport.c @@ -21,6 +21,7 @@ #include "config.h" #endif +#include #include #include #include From 5f539e9dad0b69e12f2d95661e0dcb94e1b5c769 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 13:45:49 +0200 Subject: [PATCH 13/19] Fixed missing assert include. --- libfreerdp/utils/svc_plugin.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libfreerdp/utils/svc_plugin.c b/libfreerdp/utils/svc_plugin.c index e07e8b353..4cb3d6d7a 100644 --- a/libfreerdp/utils/svc_plugin.c +++ b/libfreerdp/utils/svc_plugin.c @@ -22,6 +22,7 @@ #include "config.h" #endif +#include #include #include #include From 8442c79e106f59024c7186bc780e5208e2ae7b8a Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 14:27:34 +0200 Subject: [PATCH 14/19] Added assertions and ExitThread calls. --- channels/serial/client/serial_main.c | 5 +++++ client/X11/xf_client.c | 31 +++++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/channels/serial/client/serial_main.c b/channels/serial/client/serial_main.c index 5f8002008..f06803dea 100644 --- a/channels/serial/client/serial_main.c +++ b/channels/serial/client/serial_main.c @@ -22,6 +22,7 @@ #include "config.h" #endif +#include #include #include #include @@ -346,6 +347,7 @@ void* serial_thread_mfunc(void* arg) { SERIAL_DEVICE* serial = (SERIAL_DEVICE*)arg; + assert(NULL != serial); while(1) { int sl; @@ -366,6 +368,7 @@ void* serial_thread_mfunc(void* arg) } } + ExitThread(0); return NULL; } @@ -376,6 +379,7 @@ static void* serial_thread_func(void* arg) SERIAL_DEVICE* serial = (SERIAL_DEVICE*)arg; HANDLE ev[] = {serial->stopEvent, Queue_Event(serial->queue), serial->newEvent}; + assert(NULL != serial); while (1) { status = WaitForMultipleObjects(3, ev, FALSE, INFINITE); @@ -401,6 +405,7 @@ static void* serial_thread_func(void* arg) serial_check_fds(serial); } + ExitThread(0); return NULL; } diff --git a/client/X11/xf_client.c b/client/X11/xf_client.c index 886c3a8d2..9151bb4c8 100644 --- a/client/X11/xf_client.c +++ b/client/X11/xf_client.c @@ -773,12 +773,12 @@ BOOL xf_pre_connect(freerdp* instance) if (settings->Username == NULL) { fprintf(stderr, "--authonly, but no -u username. Please provide one.\n"); - exit(1); + return FALSE; } if (settings->Password == NULL) { fprintf(stderr, "--authonly, but no -p password. Please provide one.\n"); - exit(1); + return FALSE; } fprintf(stderr, "%s:%d: Authentication only. Don't connect to X.\n", __FILE__, __LINE__); /* Avoid XWindows initialization and configuration below. */ @@ -1224,6 +1224,8 @@ void* xf_update_thread(void* arg) wMessageQueue* queue; freerdp* instance = (freerdp*) arg; + assert( NULL != instance); + status = 1; queue = freerdp_get_message_queue(instance, FREERDP_UPDATE_MESSAGE_QUEUE); @@ -1241,6 +1243,7 @@ void* xf_update_thread(void* arg) break; } + ExitThread(0); return NULL; } @@ -1253,8 +1256,10 @@ void* xf_input_thread(void* arg) int pending_status = 1; int process_status = 1; freerdp* instance = (freerdp*) arg; + assert(NULL != instance); xfc = (xfContext*) instance->context; + assert(NULL != xfc); event = CreateFileDescriptorEvent(NULL, FALSE, FALSE, xfc->xfds); @@ -1291,6 +1296,7 @@ void* xf_input_thread(void* arg) queue = freerdp_get_message_queue(instance, FREERDP_INPUT_MESSAGE_QUEUE); MessageQueue_PostQuit(queue, 0); + ExitThread(0); return NULL; } @@ -1301,8 +1307,10 @@ void* xf_channels_thread(void* arg) HANDLE event; rdpChannels* channels; freerdp* instance = (freerdp*) arg; + assert(NULL != instance); xfc = (xfContext*) instance->context; + assert(NULL != xfc); channels = instance->context->channels; event = freerdp_channels_get_event_handle(instance); @@ -1313,6 +1321,7 @@ void* xf_channels_thread(void* arg) xf_process_channel_event(channels, instance); } + ExitThread(0); return NULL; } @@ -1356,6 +1365,7 @@ void* xf_thread(void* param) input_event = NULL; instance = (freerdp*) param; + assert(NULL != instance); ZeroMemory(rfds, sizeof(rfds)); ZeroMemory(wfds, sizeof(wfds)); @@ -1364,6 +1374,7 @@ void* xf_thread(void* param) status = freerdp_connect(instance); xfc = (xfContext*) instance->context; + assert(NULL != xfc); /* Connection succeeded. --authonly ? */ if (instance->settings->AuthenticationOnly) @@ -1535,6 +1546,18 @@ void* xf_thread(void* param) CloseHandle(update_thread); } + if (async_input) + { + WaitForSingleObject(input_thread, INFINITE); + CloseHandle(input_thread); + } + + if (async_channels) + { + WaitForSingleObject(channels_thread, INFINITE); + CloseHandle(channels_thread); + } + FILE* fin = fopen("/tmp/tsmf.tid", "rt"); if (fin) @@ -1707,7 +1730,8 @@ int xfreerdp_client_start(rdpContext* context) return -1; } - xfc->thread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE) xf_thread, context->instance, 0, NULL); + xfc->thread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE) xf_thread, + context->instance, 0, NULL); return 0; } @@ -1716,6 +1740,7 @@ int xfreerdp_client_stop(rdpContext* context) { xfContext* xfc = (xfContext*) context; + assert(NULL != context); if (context->settings->AsyncInput) { wMessageQueue* queue; From 4c1708e2906d6debdcfda9662d0d77a39e9567d1 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 14:36:06 +0200 Subject: [PATCH 15/19] Fixed channel closing before waiting for thread to quit. --- client/X11/xf_client.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client/X11/xf_client.c b/client/X11/xf_client.c index 9151bb4c8..249f57af1 100644 --- a/client/X11/xf_client.c +++ b/client/X11/xf_client.c @@ -1548,10 +1548,13 @@ void* xf_thread(void* param) if (async_input) { + wMessageQueue* input_queue = freerdp_get_message_queue(instance, FREERDP_INPUT_MESSAGE_QUEUE); + MessageQueue_PostQuit(input_queue, 0); WaitForSingleObject(input_thread, INFINITE); CloseHandle(input_thread); } + freerdp_channels_close(channels, instance); if (async_channels) { WaitForSingleObject(channels_thread, INFINITE); @@ -1594,7 +1597,6 @@ void* xf_thread(void* param) if (!exit_code) exit_code = freerdp_error_info(instance); - freerdp_channels_close(channels, instance); freerdp_channels_free(channels); freerdp_disconnect(instance); gdi_free(instance); From f2bc161f1301c4b5e9e095128a0eca1b2808e788 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 14:44:35 +0200 Subject: [PATCH 16/19] Fixed duplicate WaitForSingleObject on client thread. --- client/X11/xf_client.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/client/X11/xf_client.c b/client/X11/xf_client.c index 249f57af1..df2161e55 100644 --- a/client/X11/xf_client.c +++ b/client/X11/xf_client.c @@ -1756,8 +1756,11 @@ int xfreerdp_client_stop(rdpContext* context) xfc->disconnect = TRUE; } - WaitForSingleObject(xfc->thread, INFINITE); - CloseHandle(xfc->thread); + if (xfc->thread) + { + CloseHandle(xfc->thread); + xfc->thread = NULL; + } return 0; } From 9a3b4ac6c22e5f56cd6bb025f0603210b0a5a2e6 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 5 Sep 2013 17:17:54 +0200 Subject: [PATCH 17/19] Fixed invalid event number. --- libfreerdp/core/transport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libfreerdp/core/transport.c b/libfreerdp/core/transport.c index 933ad428b..e06e92893 100644 --- a/libfreerdp/core/transport.c +++ b/libfreerdp/core/transport.c @@ -817,11 +817,11 @@ static void* transport_client_thread(void* arg) events[nCount++] = transport->stopEvent; events[nCount] = transport->connectedEvent; - status = WaitForMultipleObjects(nCount, events, FALSE, INFINITE); + status = WaitForMultipleObjects(nCount + 1, events, FALSE, INFINITE); if (status == WAIT_OBJECT_0) break; - transport_get_read_handles(transport, (HANDLE*) events, &nCount); + transport_get_read_handles(transport, (HANDLE*) &events, &nCount); status = WaitForMultipleObjects(nCount, events, FALSE, INFINITE); if (status == WAIT_OBJECT_0) break; From 81a5e727d6d7c24ef3e75d6033071b24d6b1e992 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Fri, 6 Sep 2013 09:02:46 +0200 Subject: [PATCH 18/19] Added proper thread sync for input thread. Channels thread now listening to message pipe quit signal posted when shutting down the channel. --- channels/client/channels.c | 13 +++++++++++-- client/X11/xf_client.c | 19 ++++++++++++------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/channels/client/channels.c b/channels/client/channels.c index bf886dba4..c0739e653 100644 --- a/channels/client/channels.c +++ b/channels/client/channels.c @@ -631,8 +631,9 @@ FREERDP_API int freerdp_channels_send_event(rdpChannels* channels, wMessage* eve /** * called only from main thread */ -static void freerdp_channels_process_sync(rdpChannels* channels, freerdp* instance) +static int freerdp_channels_process_sync(rdpChannels* channels, freerdp* instance) { + int rc = TRUE; wMessage message; wMessage* event; rdpChannel* channel; @@ -642,7 +643,10 @@ static void freerdp_channels_process_sync(rdpChannels* channels, freerdp* instan while (MessageQueue_Peek(channels->MsgPipe->Out, &message, TRUE)) { if (message.id == WMQ_QUIT) + { + rc = FALSE; break; + } if (message.id == 0) { @@ -677,6 +681,8 @@ static void freerdp_channels_process_sync(rdpChannels* channels, freerdp* instan */ } } + + return rc; } /** @@ -730,7 +736,7 @@ int freerdp_channels_process_pending_messages(freerdp* instance) if (WaitForSingleObject(MessageQueue_Event(channels->MsgPipe->Out), 0) == WAIT_OBJECT_0) { - freerdp_channels_process_sync(channels, instance); + return freerdp_channels_process_sync(channels, instance); } return TRUE; @@ -782,4 +788,7 @@ void freerdp_channels_close(rdpChannels* channels, freerdp* instance) if (pChannelClientData->pChannelInitEventProc) pChannelClientData->pChannelInitEventProc(pChannelClientData->pInitHandle, CHANNEL_EVENT_TERMINATED, 0, 0); } + + /* Emit a quit signal to the internal message pipe. */ + MessagePipe_PostQuit(channels->MsgPipe, 0); } diff --git a/client/X11/xf_client.c b/client/X11/xf_client.c index df2161e55..a2ac2287e 100644 --- a/client/X11/xf_client.c +++ b/client/X11/xf_client.c @@ -1250,7 +1250,7 @@ void* xf_update_thread(void* arg) void* xf_input_thread(void* arg) { xfContext* xfc; - HANDLE event; + HANDLE event[2]; XEvent xevent; wMessageQueue* queue; int pending_status = 1; @@ -1261,9 +1261,11 @@ void* xf_input_thread(void* arg) xfc = (xfContext*) instance->context; assert(NULL != xfc); - event = CreateFileDescriptorEvent(NULL, FALSE, FALSE, xfc->xfds); + queue = freerdp_get_message_queue(instance, FREERDP_INPUT_MESSAGE_QUEUE); + event[0] = CreateFileDescriptorEvent(NULL, FALSE, FALSE, xfc->xfds); + event[1] = MessageQueue_Event(queue); - while (WaitForSingleObject(event, INFINITE) == WAIT_OBJECT_0) + while (WaitForMultipleObjects(2, event, FALSE, INFINITE) == WAIT_OBJECT_0) { do { @@ -1293,9 +1295,6 @@ void* xf_input_thread(void* arg) break; } - queue = freerdp_get_message_queue(instance, FREERDP_INPUT_MESSAGE_QUEUE); - MessageQueue_PostQuit(queue, 0); - ExitThread(0); return NULL; } @@ -1318,6 +1317,9 @@ void* xf_channels_thread(void* arg) while (WaitForSingleObject(event, INFINITE) == WAIT_OBJECT_0) { status = freerdp_channels_process_pending_messages(instance); + if (!status) + break; + xf_process_channel_event(channels, instance); } @@ -1538,6 +1540,10 @@ void* xf_thread(void* param) } } + /* Close the channels first. This will signal the internal message pipes + * that the threads should quit. */ + freerdp_channels_close(channels, instance); + if (async_update) { wMessageQueue* update_queue = freerdp_get_message_queue(instance, FREERDP_UPDATE_MESSAGE_QUEUE); @@ -1554,7 +1560,6 @@ void* xf_thread(void* param) CloseHandle(input_thread); } - freerdp_channels_close(channels, instance); if (async_channels) { WaitForSingleObject(channels_thread, INFINITE); From f39995103113651ee68f65e0d1f4ad7655167d50 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Fri, 6 Sep 2013 09:41:00 +0200 Subject: [PATCH 19/19] Fixed input thread shutdown caused by previous commit. --- client/X11/xf_client.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/X11/xf_client.c b/client/X11/xf_client.c index a2ac2287e..98ad7f3df 100644 --- a/client/X11/xf_client.c +++ b/client/X11/xf_client.c @@ -1250,7 +1250,7 @@ void* xf_update_thread(void* arg) void* xf_input_thread(void* arg) { xfContext* xfc; - HANDLE event[2]; + HANDLE event; XEvent xevent; wMessageQueue* queue; int pending_status = 1; @@ -1262,10 +1262,9 @@ void* xf_input_thread(void* arg) assert(NULL != xfc); queue = freerdp_get_message_queue(instance, FREERDP_INPUT_MESSAGE_QUEUE); - event[0] = CreateFileDescriptorEvent(NULL, FALSE, FALSE, xfc->xfds); - event[1] = MessageQueue_Event(queue); + event = CreateFileDescriptorEvent(NULL, FALSE, FALSE, xfc->xfds); - while (WaitForMultipleObjects(2, event, FALSE, INFINITE) == WAIT_OBJECT_0) + while (WaitForSingleObject(event, INFINITE) == WAIT_OBJECT_0) { do { @@ -1295,6 +1294,7 @@ void* xf_input_thread(void* arg) break; } + MessageQueue_PostQuit(queue, 0); ExitThread(0); return NULL; }