From 1aeca8fbc7366d253a63b2a34acfdba9b3a012ee Mon Sep 17 00:00:00 2001
From: Emmanuel Ledoux <eledoux@hp.com>
Date: Tue, 27 May 2014 16:17:47 +0200
Subject: [PATCH] - serial: terminates IRP threads more gracefully and avoiding
 warning messages.

---
 channels/serial/client/serial_main.c  | 36 ++++++++++++++++++++++++++-
 winpr/libwinpr/comm/comm.h            |  3 ++-
 winpr/libwinpr/comm/comm_serial_sys.c | 15 +++++++++++
 winpr/libwinpr/handle/handle.c        | 10 ++++----
 4 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/channels/serial/client/serial_main.c b/channels/serial/client/serial_main.c
index 0a9348857..ae077535f 100644
--- a/channels/serial/client/serial_main.c
+++ b/channels/serial/client/serial_main.c
@@ -50,6 +50,8 @@
 #include <winpr/thread.h>
 #include <winpr/wlog.h>
 
+#include <winpr/wlog.h>
+
 #include <freerdp/freerdp.h>
 #include <freerdp/utils/list.h>
 #include <freerdp/channels/rdpdr.h>
@@ -696,6 +698,35 @@ static void create_irp_thread(SERIAL_DEVICE *serial, IRP *irp)
 }
 
 
+static void terminate_pending_irp_threads(SERIAL_DEVICE *serial)
+{
+	ULONG_PTR *ids;
+	int i, nbIds;
+
+	nbIds = ListDictionary_GetKeys(serial->IrpThreads, &ids);
+
+	DEBUG_SVC("Terminating %d IRP thread(s)", nbIds);
+
+	for (i=0; i<nbIds; i++)
+	{
+		HANDLE irpThread;
+		ULONG_PTR id = ids[i];
+
+		irpThread = ListDictionary_GetItemValue(serial->IrpThreads, (void*)id);		
+
+		TerminateThread(irpThread, 0);
+
+		WaitForSingleObject(irpThread, INFINITE);
+
+		CloseHandle(irpThread);
+
+		DEBUG_SVC("IRP thread terminated, CompletionId %d", id);
+	}
+
+	ListDictionary_Clear(serial->IrpThreads);
+}
+
+
 static void* serial_thread_func(void* arg)
 {
 	IRP* irp;
@@ -711,7 +742,10 @@ static void* serial_thread_func(void* arg)
 			break;
 
 		if (message.id == WMQ_QUIT)
+		{
+			terminate_pending_irp_threads(serial);
 			break;
+		}
 
 		irp = (IRP*) message.wParam;
 
@@ -748,7 +782,7 @@ static void serial_free(DEVICE* device)
 	WLog_Print(serial->log, WLOG_DEBUG, "freeing");
 
 	MessageQueue_PostQuit(serial->MainIrpQueue, 0);
-	WaitForSingleObject(serial->MainThread, INFINITE); /* FIXME: might likely block on a pending Write or ioctl */
+	WaitForSingleObject(serial->MainThread, INFINITE);
 	CloseHandle(serial->MainThread);
 
 	if (serial->hComm)
diff --git a/winpr/libwinpr/comm/comm.h b/winpr/libwinpr/comm/comm.h
index aac9e7a52..b382cdfa7 100644
--- a/winpr/libwinpr/comm/comm.h
+++ b/winpr/libwinpr/comm/comm.h
@@ -74,7 +74,8 @@ typedef struct winpr_comm WINPR_COMM;
 void _comm_setRemoteSerialDriver(HANDLE hComm, REMOTE_SERIAL_DRIVER_ID);
 
 /* TMP: TODO: move all specific defines and types here? at least SERIAL_EV_* */
-#define SERIAL_EV_FREERDP_STOP     0x8000 /* bit unused by SERIAL_EV_* */
+#define SERIAL_EV_FREERDP_STOP     0x4000 /* bit unused by SERIAL_EV_* */
+#define SERIAL_EV_FREERDP_CLOSING  0x8000 /* bit unused by SERIAL_EV_* */
 
 
 #endif /* _WIN32 */
diff --git a/winpr/libwinpr/comm/comm_serial_sys.c b/winpr/libwinpr/comm/comm_serial_sys.c
index 30792c792..572cb6c86 100644
--- a/winpr/libwinpr/comm/comm_serial_sys.c
+++ b/winpr/libwinpr/comm/comm_serial_sys.c
@@ -1016,6 +1016,11 @@ static BOOL _set_wait_mask(WINPR_COMM *pComm, const ULONG *pWaitMask)
 	/* NB: ensure to leave the critical section before to return */
 	EnterCriticalSection(&pComm->EventsLock);
 
+	if (pComm->PendingEvents |= SERIAL_EV_FREERDP_CLOSING)
+	{
+		return TRUE; /* returns without complaining */
+	}
+
 	if (*pWaitMask == 0)
 	{
 		/* clearing pending events */
@@ -1165,6 +1170,11 @@ static BOOL _get_commstatus(WINPR_COMM *pComm, SERIAL_STATUS *pCommstatus)
 	/* NB: ensure to leave the critical section before to return */
 	EnterCriticalSection(&pComm->EventsLock);
 
+	if (pComm->PendingEvents & SERIAL_EV_FREERDP_CLOSING)
+	{
+		return TRUE; /* returns without complaining */
+	}
+
 	ZeroMemory(pCommstatus, sizeof(SERIAL_STATUS));
 
 	ZeroMemory(&currentCounters, sizeof(struct serial_icounter_struct));
@@ -1341,6 +1351,11 @@ static BOOL _wait_on_mask(WINPR_COMM *pComm, ULONG *pOutputMask)
 
 		/* NB: ensure to leave the critical section before to return */
 		EnterCriticalSection(&pComm->EventsLock);
+		
+		if (pComm->PendingEvents & SERIAL_EV_FREERDP_CLOSING)
+		{
+			return TRUE; /* returns without complaining */
+		}
 
 		if (pComm->PendingEvents & SERIAL_EV_FREERDP_STOP)
 		{
diff --git a/winpr/libwinpr/handle/handle.c b/winpr/libwinpr/handle/handle.c
index ca5d8bae1..4d777d265 100644
--- a/winpr/libwinpr/handle/handle.c
+++ b/winpr/libwinpr/handle/handle.c
@@ -203,19 +203,19 @@ BOOL CloseHandle(HANDLE hObject)
 
 		comm = (WINPR_COMM*) Object;
 
-		if (comm->fd > 0)
-			close(comm->fd);
-
 		/* NOTE: This is up to the caller of CloseHandle() to
 		 * ensure there is no pending request. Sending
 		 * SERIAL_EV_FREERDP_STOP anyway. Remove this code if
 		 * you think otherwise. */
 		EnterCriticalSection(&comm->EventsLock);
-		comm->PendingEvents |= SERIAL_EV_FREERDP_STOP;
+		comm->PendingEvents |= SERIAL_EV_FREERDP_CLOSING;
 		LeaveCriticalSection(&comm->EventsLock);
-		
+
 		DeleteCriticalSection(&comm->EventsLock);
 
+		if (comm->fd > 0)
+			close(comm->fd);
+
 		free(comm);
 
 		return TRUE;