From ac63b9ae566b9b5d2ebc60adf876ebde0b110318 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 13 Aug 2013 14:04:17 +0200 Subject: [PATCH 1/6] Using WaitForMultipleObjects now to reduce CPU load. --- channels/serial/client/serial_main.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/channels/serial/client/serial_main.c b/channels/serial/client/serial_main.c index fb6b07915..80be6c88f 100644 --- a/channels/serial/client/serial_main.c +++ b/channels/serial/client/serial_main.c @@ -323,15 +323,13 @@ static void* serial_thread_func(void* arg) IRP* irp; DWORD status; SERIAL_DEVICE* serial = (SERIAL_DEVICE*)arg; + HANDLE ev[] = {serial->stopEvent, Queue_Event(serial->queue)}; while (1) { - if (WaitForSingleObject(serial->stopEvent, 0) == WAIT_OBJECT_0) - break; + status = WaitForMultipleObjects(2, ev, FALSE, 10); - status = WaitForSingleObject(Queue_Event(serial->queue), 10); - - if ((status != WAIT_OBJECT_0) && (status != WAIT_TIMEOUT)) + if (WAIT_OBJECT_0 == status) break; serial->nfds = 1; @@ -342,7 +340,7 @@ static void* serial_thread_func(void* arg) serial->tv.tv_usec = 0; serial->select_timeout = 0; - if (status == WAIT_OBJECT_0) + if (status == WAIT_OBJECT_0 + 1) { if ((irp = (IRP*) Queue_Dequeue(serial->queue))) serial_process_irp(serial, irp); From 86c0c0297519375edcc4d9c8106fed08dc58987c Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 13 Aug 2013 16:04:19 +0200 Subject: [PATCH 2/6] Fixed resource leaks. --- channels/parallel/client/parallel_main.c | 1 + channels/serial/client/serial_main.c | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/channels/parallel/client/parallel_main.c b/channels/parallel/client/parallel_main.c index 6e35c1ee9..16bf4bbb0 100644 --- a/channels/parallel/client/parallel_main.c +++ b/channels/parallel/client/parallel_main.c @@ -286,6 +286,7 @@ static void parallel_free(DEVICE* device) MessageQueue_PostQuit(parallel->queue, 0); WaitForSingleObject(parallel->thread, INFINITE); + Stream_Free(parallel->device.data, TRUE); MessageQueue_Free(parallel->queue); CloseHandle(parallel->thread); diff --git a/channels/serial/client/serial_main.c b/channels/serial/client/serial_main.c index 80be6c88f..cb0852a5a 100644 --- a/channels/serial/client/serial_main.c +++ b/channels/serial/client/serial_main.c @@ -365,10 +365,16 @@ static void serial_free(DEVICE* device) DEBUG_SVC("freeing device"); + /* Stop thread */ SetEvent(serial->stopEvent); + WaitForSingleObject(serial->thread, INFINITE); - /* TODO: free lists */ - + /* Clean up resources */ + Stream_Free(serial->device.data, TRUE); + Queue_Free(serial->queue); + list_free(serial->pending_irps); + CloseHandle(serial->stopEvent); + CloseHandle(serial->thread); free(serial); } From 605f956486dcdcf997c06dfd4c80362b01bf798d Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 13 Aug 2013 16:29:19 +0200 Subject: [PATCH 3/6] Fixed resource leaks and missing thread sync. --- channels/smartcard/client/smartcard_main.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/channels/smartcard/client/smartcard_main.c b/channels/smartcard/client/smartcard_main.c index fa4dd0b3d..4efff88ff 100644 --- a/channels/smartcard/client/smartcard_main.c +++ b/channels/smartcard/client/smartcard_main.c @@ -44,8 +44,7 @@ static void smartcard_free(DEVICE* dev) SMARTCARD_DEVICE* smartcard = (SMARTCARD_DEVICE*) dev; SetEvent(smartcard->stopEvent); - CloseHandle(smartcard->thread); - CloseHandle(smartcard->irpEvent); + WaitForSingleObject(smartcard->thread, INFINITE); while ((irp = (IRP*) InterlockedPopEntrySList(smartcard->pIrpList)) != NULL) irp->Discard(irp); @@ -55,8 +54,14 @@ static void smartcard_free(DEVICE* dev) /* Begin TS Client defect workaround. */ while ((CompletionIdInfo = (COMPLETIONIDINFO*) list_dequeue(smartcard->CompletionIds)) != NULL) - free(CompletionIdInfo); + free(CompletionIdInfo); + CloseHandle(smartcard->thread); + CloseHandle(smartcard->irpEvent); + CloseHandle(smartcard->stopEvent); + CloseHandle(smartcard->CompletionIdsMutex); + + Stream_Free(smartcard->device.data, TRUE); list_free(smartcard->CompletionIds); /* End TS Client defect workaround. */ @@ -119,13 +124,16 @@ static void smartcard_process_irp_thread_func(SMARTCARD_IRP_WORKER* irpWorker) static void* smartcard_thread_func(void* arg) { SMARTCARD_DEVICE* smartcard = (SMARTCARD_DEVICE*) arg; + HANDLE ev[] = {smartcard->irpEvent, smartcard->stopEvent}; while (1) { - WaitForSingleObject(smartcard->irpEvent, INFINITE); + DWORD status = WaitForSingleObject(2, ev, FALSE, INFINITE); - if (WaitForSingleObject(smartcard->stopEvent, 0) == WAIT_OBJECT_0) + if (status == WAIT_OBJECT_0 + 1) break; + else if(status != WAIT_OBJECT_0) + continue; ResetEvent(smartcard->irpEvent); smartcard_process_irp_list(smartcard); From 4a5b19e8163550c5d3c08b17237f758f1ce08df0 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Wed, 14 Aug 2013 12:13:48 +0200 Subject: [PATCH 4/6] Fixed high CPU usage. --- channels/serial/client/serial_main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/channels/serial/client/serial_main.c b/channels/serial/client/serial_main.c index cb0852a5a..d62ca7a91 100644 --- a/channels/serial/client/serial_main.c +++ b/channels/serial/client/serial_main.c @@ -327,7 +327,7 @@ static void* serial_thread_func(void* arg) while (1) { - status = WaitForMultipleObjects(2, ev, FALSE, 10); + status = WaitForMultipleObjects(2, ev, FALSE, 1); if (WAIT_OBJECT_0 == status) break; @@ -336,7 +336,7 @@ static void* serial_thread_func(void* arg) FD_ZERO(&serial->read_fds); FD_ZERO(&serial->write_fds); - serial->tv.tv_sec = 1; + serial->tv.tv_sec = 0; serial->tv.tv_usec = 0; serial->select_timeout = 0; @@ -344,6 +344,7 @@ static void* serial_thread_func(void* arg) { if ((irp = (IRP*) Queue_Dequeue(serial->queue))) serial_process_irp(serial, irp); + continue; } serial_check_fds(serial); From cf6b9d44ac4d3b512af1d090f72c6f6006716f42 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Wed, 14 Aug 2013 15:14:40 +0200 Subject: [PATCH 5/6] Fixed invalid access to tty in thread, which was already removed by serial_process_irp_close Retry read now, if non blocking IO returns EAGAIN. --- channels/serial/client/serial_main.c | 35 +++++++++++++++++++++++++++- channels/serial/client/serial_tty.c | 14 ++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/channels/serial/client/serial_main.c b/channels/serial/client/serial_main.c index d62ca7a91..d2ff16a48 100644 --- a/channels/serial/client/serial_main.c +++ b/channels/serial/client/serial_main.c @@ -370,6 +370,8 @@ static void serial_free(DEVICE* device) SetEvent(serial->stopEvent); WaitForSingleObject(serial->thread, INFINITE); + serial_tty_free(serial->tty); + /* Clean up resources */ Stream_Free(serial->device.data, TRUE); Queue_Free(serial->queue); @@ -388,6 +390,11 @@ static void serial_abort_single_io(SERIAL_DEVICE* serial, UINT32 file_id, UINT32 DEBUG_SVC("[in] pending size %d", list_size(serial->pending_irps)); tty = serial->tty; + if(!tty) + { + DEBUG_WARN("tty = %p", tty); + return; + } switch (abort_io) { @@ -438,6 +445,11 @@ static void serial_check_for_events(SERIAL_DEVICE* serial) SERIAL_TTY* tty; tty = serial->tty; + if(!tty) + { + DEBUG_WARN("tty = %p", tty); + return; + } DEBUG_SVC("[in] pending size %d", list_size(serial->pending_irps)); @@ -483,6 +495,11 @@ void serial_get_timeouts(SERIAL_DEVICE* serial, IRP* irp, UINT32* timeout, UINT3 DEBUG_SVC("length read %u", Length); tty = serial->tty; + if(!tty) + { + DEBUG_WARN("tty = %p", tty); + return; + } *timeout = (tty->read_total_timeout_multiplier * Length) + tty->read_total_timeout_constant; *interval_timeout = tty->read_interval_timeout; @@ -497,6 +514,11 @@ static void serial_handle_async_irp(SERIAL_DEVICE* serial, IRP* irp) SERIAL_TTY* tty; tty = serial->tty; + if(!tty) + { + DEBUG_WARN("tty = %p", tty); + return; + } switch (irp->MajorFunction) { @@ -547,6 +569,11 @@ static void __serial_check_fds(SERIAL_DEVICE* serial) ZeroMemory(&serial->tv, sizeof(struct timeval)); tty = serial->tty; + if(!tty) + { + DEBUG_WARN("tty = %p", tty); + return; + } /* scan every pending */ irp = list_peek(serial->pending_irps); @@ -609,6 +636,11 @@ static void serial_set_fds(SERIAL_DEVICE* serial) DEBUG_SVC("[in] pending size %d", list_size(serial->pending_irps)); tty = serial->tty; + if(!tty) + { + DEBUG_WARN("tty = %p", tty); + return; + } irp = (IRP*) list_peek(serial->pending_irps); while (irp) @@ -710,7 +742,8 @@ int DeviceServiceEntry(PDEVICE_SERVICE_ENTRY_POINTS pEntryPoints) pEntryPoints->RegisterDevice(pEntryPoints->devman, (DEVICE*) serial); - serial->thread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE) serial_thread_func, (void*) serial, 0, NULL); + serial->thread = CreateThread(NULL, 0, + (LPTHREAD_START_ROUTINE) serial_thread_func, (void*) serial, 0, NULL); } return 0; diff --git a/channels/serial/client/serial_tty.c b/channels/serial/client/serial_tty.c index 23445b2ca..9da45bdb0 100644 --- a/channels/serial/client/serial_tty.c +++ b/channels/serial/client/serial_tty.c @@ -409,10 +409,19 @@ BOOL serial_tty_read(SERIAL_TTY* tty, BYTE* buffer, UINT32* Length) ZeroMemory(buffer, *Length); - status = read(tty->fd, buffer, *Length); + do + { + errno = 0; + status = read(tty->fd, buffer, *Length); + } + while(EAGAIN == errno); if (status < 0) + { + DEBUG_WARN("failed with %zd, errno=[%d] %s\n", + status, errno, strerror(errno)); return FALSE; + } tty->event_txempty = status; *Length = status; @@ -456,6 +465,9 @@ void serial_tty_free(SERIAL_TTY* tty) { DEBUG_SVC("in"); + if(!tty) + return; + if (tty->fd >= 0) { if (tty->pold_termios) From bd74f5c8b51736808716ffb5477e4316f08d1664 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Wed, 14 Aug 2013 17:33:46 +0200 Subject: [PATCH 6/6] Removed EAGAIN handling, again passing on the error to the server. --- channels/serial/client/serial_tty.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/channels/serial/client/serial_tty.c b/channels/serial/client/serial_tty.c index 9da45bdb0..95bf375fe 100644 --- a/channels/serial/client/serial_tty.c +++ b/channels/serial/client/serial_tty.c @@ -409,12 +409,7 @@ BOOL serial_tty_read(SERIAL_TTY* tty, BYTE* buffer, UINT32* Length) ZeroMemory(buffer, *Length); - do - { - errno = 0; - status = read(tty->fd, buffer, *Length); - } - while(EAGAIN == errno); + status = read(tty->fd, buffer, *Length); if (status < 0) {