- winpr-comm: fixed again the synchronization arround PendingEvents. Prefer to wait for the end of _wait_on_mask() inside _set_wait_mask()

This commit is contained in:
Emmanuel Ledoux 2014-05-27 17:29:55 +02:00 committed by Emmanuel Ledoux
parent ae3dd68b88
commit b8d00e41c4
3 changed files with 36 additions and 25 deletions

View File

@ -74,8 +74,8 @@ typedef struct winpr_comm WINPR_COMM;
void _comm_setRemoteSerialDriver(HANDLE hComm, REMOTE_SERIAL_DRIVER_ID); void _comm_setRemoteSerialDriver(HANDLE hComm, REMOTE_SERIAL_DRIVER_ID);
/* TMP: TODO: move all specific defines and types here? at least SERIAL_EV_* */ /* TMP: TODO: move all specific defines and types here? at least SERIAL_EV_* */
#define SERIAL_EV_FREERDP_STOP 0x4000 /* bit unused by SERIAL_EV_* */ #define SERIAL_EV_FREERDP_WAITING 0x4000 /* bit unused by SERIAL_EV_* */
#define SERIAL_EV_FREERDP_CLOSING 0x8000 /* bit unused by SERIAL_EV_* */ #define SERIAL_EV_FREERDP_STOP 0x8000 /* bit unused by SERIAL_EV_* */
#endif /* _WIN32 */ #endif /* _WIN32 */

View File

@ -1013,14 +1013,27 @@ static BOOL _set_wait_mask(WINPR_COMM *pComm, const ULONG *pWaitMask)
{ {
ULONG possibleMask; ULONG possibleMask;
/* Stops pending IOCTL_SERIAL_WAIT_ON_MASK
* http://msdn.microsoft.com/en-us/library/ff546805%28v=vs.85%29.aspx
*/
if (pComm->PendingEvents & SERIAL_EV_FREERDP_WAITING)
{
/* FIXME: any doubt on reading PendingEvents out of a critical section? */
EnterCriticalSection(&pComm->EventsLock);
pComm->PendingEvents |= SERIAL_EV_FREERDP_STOP;
LeaveCriticalSection(&pComm->EventsLock);
/* waiting the end of the pending _wait_on_mask() */
while (pComm->PendingEvents & SERIAL_EV_FREERDP_WAITING)
Sleep(10); /* 10ms */
}
/* NB: ensure to leave the critical section before to return */ /* NB: ensure to leave the critical section before to return */
EnterCriticalSection(&pComm->EventsLock); EnterCriticalSection(&pComm->EventsLock);
if (pComm->PendingEvents & SERIAL_EV_FREERDP_CLOSING)
{
return TRUE; /* returns without complaining */
}
if (*pWaitMask == 0) if (*pWaitMask == 0)
{ {
/* clearing pending events */ /* clearing pending events */
@ -1037,11 +1050,6 @@ static BOOL _set_wait_mask(WINPR_COMM *pComm, const ULONG *pWaitMask)
pComm->PendingEvents = 0; pComm->PendingEvents = 0;
} }
/* Stops pending IOCTL_SERIAL_WAIT_ON_MASK
* http://msdn.microsoft.com/en-us/library/ff546805%28v=vs.85%29.aspx
*/
pComm->PendingEvents |= SERIAL_EV_FREERDP_STOP;
possibleMask = *pWaitMask & _SERIAL_SYS_SUPPORTED_EV_MASK; possibleMask = *pWaitMask & _SERIAL_SYS_SUPPORTED_EV_MASK;
if (possibleMask != *pWaitMask) if (possibleMask != *pWaitMask)
@ -1170,11 +1178,6 @@ static BOOL _get_commstatus(WINPR_COMM *pComm, SERIAL_STATUS *pCommstatus)
/* NB: ensure to leave the critical section before to return */ /* NB: ensure to leave the critical section before to return */
EnterCriticalSection(&pComm->EventsLock); EnterCriticalSection(&pComm->EventsLock);
if (pComm->PendingEvents & SERIAL_EV_FREERDP_CLOSING)
{
return TRUE; /* returns without complaining */
}
ZeroMemory(pCommstatus, sizeof(SERIAL_STATUS)); ZeroMemory(pCommstatus, sizeof(SERIAL_STATUS));
ZeroMemory(&currentCounters, sizeof(struct serial_icounter_struct)); ZeroMemory(&currentCounters, sizeof(struct serial_icounter_struct));
@ -1338,25 +1341,25 @@ static BOOL _wait_on_mask(WINPR_COMM *pComm, ULONG *pOutputMask)
{ {
assert(*pOutputMask == 0); assert(*pOutputMask == 0);
/* UGLY: removes the STOP bit set by an initial _set_wait_mask() */ EnterCriticalSection(&pComm->EventsLock);
pComm->PendingEvents &= ~SERIAL_EV_FREERDP_STOP; pComm->PendingEvents |= SERIAL_EV_FREERDP_WAITING;
LeaveCriticalSection(&pComm->EventsLock);
while (TRUE) while (TRUE)
{ {
/* NB: EventsLock also used by _refresh_PendingEvents() */ /* NB: EventsLock also used by _refresh_PendingEvents() */
if (!_refresh_PendingEvents(pComm)) if (!_refresh_PendingEvents(pComm))
{ {
EnterCriticalSection(&pComm->EventsLock);
pComm->PendingEvents &= ~SERIAL_EV_FREERDP_WAITING;
LeaveCriticalSection(&pComm->EventsLock);
return FALSE; return FALSE;
} }
/* NB: ensure to leave the critical section before to return */ /* NB: ensure to leave the critical section before to return */
EnterCriticalSection(&pComm->EventsLock); EnterCriticalSection(&pComm->EventsLock);
if (pComm->PendingEvents & SERIAL_EV_FREERDP_CLOSING)
{
return TRUE; /* returns without complaining */
}
if (pComm->PendingEvents & SERIAL_EV_FREERDP_STOP) if (pComm->PendingEvents & SERIAL_EV_FREERDP_STOP)
{ {
pComm->PendingEvents &= ~SERIAL_EV_FREERDP_STOP; pComm->PendingEvents &= ~SERIAL_EV_FREERDP_STOP;
@ -1368,6 +1371,7 @@ static BOOL _wait_on_mask(WINPR_COMM *pComm, ULONG *pOutputMask)
*/ */
assert(*pOutputMask == 0); assert(*pOutputMask == 0);
pComm->PendingEvents &= ~SERIAL_EV_FREERDP_WAITING;
LeaveCriticalSection(&pComm->EventsLock); LeaveCriticalSection(&pComm->EventsLock);
return TRUE; return TRUE;
} }
@ -1391,6 +1395,10 @@ static BOOL _wait_on_mask(WINPR_COMM *pComm, ULONG *pOutputMask)
if (*pOutputMask != 0) if (*pOutputMask != 0)
{ {
/* at least an event occurred */ /* at least an event occurred */
EnterCriticalSection(&pComm->EventsLock);
pComm->PendingEvents &= ~SERIAL_EV_FREERDP_WAITING;
LeaveCriticalSection(&pComm->EventsLock);
return TRUE; return TRUE;
} }
@ -1407,6 +1415,9 @@ static BOOL _wait_on_mask(WINPR_COMM *pComm, ULONG *pOutputMask)
} }
DEBUG_WARN("_wait_on_mask, unexpected return, WaitEventMask=0X%lX", pComm->WaitEventMask); DEBUG_WARN("_wait_on_mask, unexpected return, WaitEventMask=0X%lX", pComm->WaitEventMask);
EnterCriticalSection(&pComm->EventsLock);
pComm->PendingEvents &= ~SERIAL_EV_FREERDP_WAITING;
LeaveCriticalSection(&pComm->EventsLock);
assert(FALSE); assert(FALSE);
return FALSE; return FALSE;
} }

View File

@ -208,7 +208,7 @@ BOOL CloseHandle(HANDLE hObject)
* SERIAL_EV_FREERDP_STOP anyway. Remove this code if * SERIAL_EV_FREERDP_STOP anyway. Remove this code if
* you think otherwise. */ * you think otherwise. */
EnterCriticalSection(&comm->EventsLock); EnterCriticalSection(&comm->EventsLock);
comm->PendingEvents |= SERIAL_EV_FREERDP_CLOSING; comm->PendingEvents |= SERIAL_EV_FREERDP_STOP;
LeaveCriticalSection(&comm->EventsLock); LeaveCriticalSection(&comm->EventsLock);
DeleteCriticalSection(&comm->EventsLock); DeleteCriticalSection(&comm->EventsLock);