poco icon indicating copy to clipboard operation
poco copied to clipboard

Poco EventImpl for Win32 breaks INFINITE wait

Open Coder666 opened this issue 2 years ago • 5 comments

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.

Coder666 avatar Aug 09 '22 13:08 Coder666

another way of looking at this is that it is an integer overflow bug

Coder666 avatar Aug 09 '22 14:08 Coder666

@obiltschnig what is the reason for adding 1 here?

https://github.com/pocoproject/poco/blob/be19dc4a2f30eb97cc9bdd7551460db11cc27353/Foundation/src/Event_WIN32.cpp#L49

aleks-f avatar Aug 10 '22 11:08 aleks-f

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 ;-)

obiltschnig avatar Aug 10 '22 11:08 obiltschnig

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)

Coder666 avatar Aug 10 '22 11:08 Coder666

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.

Coder666 avatar Aug 10 '22 12:08 Coder666