poco
poco copied to clipboard
Poco EventImpl for Win32 breaks INFINITE wait
Describe the bug
EventImpl::waitImpl(long)
adds one to the timeout value. If you specify "INFINITE" which is DWORD(-1)/0xFFFFFFFF then this waits for 0 which is completly unexpected behaviour
To Reproduce
Create an event object on Windows and wait for INFINITE
Expected behavior
When -1 is specifed the wait should blocks until signalled or error, or at the very least it should wait for 0xFFFFFFFF milliseconds!
Proposed Fixes
- Add a constant to Event for infinite
- Add an assert on Win32 that checks if "INFINITE" has been used
- Stop adding 1 to the value.
another way of looking at this is that it is an integer overflow bug
@obiltschnig what is the reason for adding 1 here?
https://github.com/pocoproject/poco/blob/be19dc4a2f30eb97cc9bdd7551460db11cc27353/Foundation/src/Event_WIN32.cpp#L49
I no longer remember. It seems to be that way since the 0.9 releases. I'd remove the + 1
and see if anyone complains ;-)
I removed the plus one and had various issues with threads spinning/churning CPU time.
I didn't look into the why's of it, however it does seem like there is perhaps a WaitForSingleObject(0) issue. If you read the documentation:
If dwMilliseconds is zero, the function does not enter a wait state if the object is not signaled; it always returns immediately.
https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitforsingleobject
So perhaps a +1 was added to prevent WaitForSingleObject(0) ? (i.e calling WaitForSingleObject(0) in a loop will not yield the thread)
Note that I just changed it to the following to test this theory:
bool EventImpl::waitImpl(long milliseconds)
{
_ASSERT(milliseconds != INFINITE);
switch (WaitForSingleObject(_event, milliseconds==0 ? 1 : milliseconds ))
{
case WAIT_TIMEOUT:
return false;
case WAIT_OBJECT_0:
return true;
default:
throw SystemException("wait for event failed");
}
}
The thread spinning issue did not occur.