WaitForMultipleObjects never wakes up - Windows-bug?

[Bonita Montero]

I've written a very efficient condvar-implementation for Win32.
Here it is: http://pastebin.com/fdjHx9ec
I'm pretty sure it's considerably more efficient than the
condvars in Win32 since Vista and Windows Server 2008.

But I had a strange phnomenon when a thread holding the condvar
locked is singalling a waiting thread and after this is sleeping
before it is releasing the lock.
I extracted the problem in a simple programm without this kind
of efficient locking through atomic variables, but just with an
event and a semaphore. Here it is:

#include <windows.h>
#include <stdio.h>

HANDLE hEvt,
hSema;
bool volatile fReleased = false;

DWORD WINAPI LockAndReleaseThread( LPVOID lpvThreadParam );

int main()
{
int const NTHREADS = 2;
HANDLE ahWait[2];

ahWait[0] = ::hEvt = CreateEvent( NULL, FALSE, TRUE, NULL );
ahWait[1] = ::hSema = CreateSemaphore( NULL, 0, 1, NULL );

for( int i = 0; i < NTHREADS; i++ )
CreateThread( NULL, 0, LockAndReleaseThread, NULL, 0, NULL );

for( ; ; )
{
WaitForMultipleObjects( 2, ahWait, TRUE, INFINITE );
printf( "main thread is holding lock and received signal\n" );
::fReleased = false;
SetEvent( hEvt );
}

return 0;
}

DWORD WINAPI LockAndReleaseThread( LPVOID lpvThreadParam )
{
for( ; ; )
{
WaitForSingleObject( hEvt, INFINITE );
printf( "spawned thread is holding lock\n" );

if( !::fReleased )
ReleaseSemaphore( ::hSema, 1, NULL ),
::fReleased = true;

Sleep( 1000 );
SetEvent( hEvt );
}

return 0;
}

The strange thing here is, that whith my Windows 10 computer,
WaitForMultipleObjects in the main-thread never awakens!
When I set NTHREADS to 1, the main-thread gets a chance, but
beyond it will freeze.
Has anyone an explanation for that?

Thanks in advance,
Bonita Montero

--
http://facebook.com/bonita.montero/

---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus

[Bonita Montero]

I've got a solution for this problem.
In the main-thread, simply wait for the semaphore and then for the
event (in this order - or you're gonna deadlock) with WaitForSingle
Object. That's not so efficitent, but works.

int main()
{
int const NTHREADS = 2;
HANDLE ahWait[2];

::hEvt = CreateEvent( NULL, FALSE, TRUE, NULL );

::hSema = CreateSemaphore( NULL, 0, 1, NULL );

fReleased = false;

for( int i = 0; i < NTHREADS; i++ )
CreateThread( NULL, 0, LockAndReleaseThread, NULL, 0, NULL );

for( ; ; )

WaitForSingleObject( ::hSema, INFINITE ),
WaitForSingleObject( ::hEvt, INFINITE),
printf( "main thread is holding lock and received signal\n" ),
::fReleased = false,

SetEvent( ::hEvt );

return 0;
}

[JJ]

It's because ahWait will mostly be [signalled,non-signalled] or
[non-signalled,signalled] on fast PCs or in OS with more efficient thread
scheduler.

The reason is in the thread code. It leaves nothing after SetEvent() and
before WaitForSingleObject(), except the for's loop. In assembly level, this
would be a mere one CPU instruction (a jump to the loop start) plus the
overhead for calling WaitForSingleObject(). This means that the time for
hEvt to become signalled is extremely short. So, the chance of ahWait to
become [signalled,signalled] is extremely small. Considering that the test
code use only two threads and 1 second delay.

My suggestion is to move the Sleep() after SetEvent() and give it at least
1ms of sleeping time.

[Bonita Montero]

Am 23.05.2016 um 13:23 schrieb JJ:

> It's because ahWait will mostly be [signalled,non-signalled] or
> [non-signalled,signalled] on fast PCs or in OS with more efficient
> thread scheduler.

No, that has nothing to do directly with the scheduler.
When I say SetEvent, windows shows which thread to push on the runnable
list of threads. And it doesn't do that in a fair manner. It prefers
threads that wait for a single object over those where this event is
one among other handles the thread is waiting for.

> The reason is in the thread code. It leaves nothing after SetEvent()
> and before WaitForSingleObject(), except the for's loop.

When this would be a problem, the code won't run correctly when NTHREADS
is one. But it does work. And when NTHREADS is > 1, those threads won't
give each other CPU-time. But they do; the threads hand each other the
CPU-time in a round-robin fashion in a repeating order - try it.
And it does even work with NTHREADS > 1 when I replace the WaitFor
MultipleObjects with this:

WaitForSingleObject( ::hSema, INFINITE ),
WaitForSingleObject( ::hEvt, INFINITE),

Theoretically this would do the same, but practically - because of the
Windows-bug - it makes a difference.

> My suggestion is to move the Sleep() after SetEvent() and give it at least
> 1ms of sleeping time.

That's just an experiment, not code for any special purpose.
And its sufficient to show that Windows has a bug here.

[Bonita Montero]

Am 23.05.2016 um 13:23 schrieb JJ:

> The reason is in the thread code. It leaves nothing after SetEvent()

> and before WaitForSingleObject(), except the for's loop. ...

I made a little demo that shows, that this isn't true.
In the following code, according to your theory, one thread would
starve out the other thread. But it does not! The threads are put
on the runnable list when SetEvent is called and the state of the
event changes to non signalled even before SetEvent returns.


#include <windows.h>
#include <stdio.h>

DWORD WINAPI ContentionThread( LPVOID lpvThreadParam );

int main()
{
HANDLE hEvtContention;

hEvtContention = CreateEvent( NULL, FALSE, TRUE, NULL );
CreateThread( NULL, 0, ContentionThread,
(LPVOID)hEvtContention, 0, NULL );
CreateThread( NULL, 0, ContentionThread,
(LPVOID)hEvtContention, 0, NULL );
Sleep( INFINITE );

return 0;
}

DWORD WINAPI ContentionThread( LPVOID lpvThreadParam )
{
HANDLE hEvt = (HANDLE)lpvThreadParam;

for( ; ; )
WaitForSingleObject( hEvt, INFINITE ),
printf( "thread with ID %08X is running\n",
(unsigned)GetCurrentThreadId() ),
Sleep( 1000 ),

SetEvent( hEvt );

return 0;
}

[Big Bad Bob]

On 05/22/16 09:47, Bonita Montero so wittily quipped:
> WaitForSingleObject( hEvt, INFINITE );

maybe you should pick a timeout period that's relatively short, say 10
milliseconds, and poll for 'whatever else'. Also could use
MsgWaitForXXX functions, have them wake up on WM_TIMER's

[Big Bad Bob]

On 05/23/16 04:23, JJ so wittily quipped:

> My suggestion is to move the Sleep() after SetEvent() and give it at least
> 1ms of sleeping time.

correct, Sleep(0) has undesirable effects in every version of windows
I've ever seen. same with 'yield()' If you spin on those, you'll
consume 100% CPU. Sleep(1), however, doesn't consume 100% CPU because
it goes into an actual WAIT STATE.


as for locking semantics, it might be worth pointing out that locking a
set of resources is probably a BAD idea. You should design your system to

a) lock on a per-object level, and serialize important things in VERY
tight lock/unlock processes, like spinlocks or critical sections.

b) give yourself a graceful ability to bail out of a locking process
when you can't acquire ALL of the locks

c) don't require more than one lock per resource, and let it go as soon
as you can

The various 'InterlockedXXcrement' and related functions can help with
some of this, especially reference counts and spin locks.

[Bonita Montero]

Am 24.05.2016 um 10:20 schrieb Big Bad Bob:

> correct, Sleep(0) has undesirable effects in every version of windows
> I've ever seen. same with 'yield()' If you spin on those, you'll
> consume 100% CPU. Sleep(1), however, doesn't consume 100% CPU because
> it goes into an actual WAIT STATE.

That's not the point. It makes no difference if I call sleep, or if I
have a large loop.

> a) lock on a per-object level, and serialize important things in VERY
> tight lock/unlock processes, like spinlocks or critical sections.

My code was for demonstrating, that Windows prefers threads tha do
WaitForSingleObject over those that do WaitForMultipleObjects. That
an event isn't as efficient than a CRITICAL_SECTION is clear.

[Bonita Montero]

Am 24.05.2016 um 10:14 schrieb Big Bad Bob:

> maybe you should pick a timeout period that's relatively short, say 10
> milliseconds, and poll for 'whatever else'. Also could use
> MsgWaitForXXX functions, have them wake up on WM_TIMER's

I found a workaround, look here: <nhunjd$prv$2...@news.albasani.net>

[Big Bad Bob]

On 05/24/16 05:24, Bonita Montero so wittily quipped:

> Am 24.05.2016 um 10:14 schrieb Big Bad Bob:
>
>> maybe you should pick a timeout period that's relatively short, say 10
>> milliseconds, and poll for 'whatever else'. Also could use
>> MsgWaitForXXX functions, have them wake up on WM_TIMER's
>
> I found a workaround, look here: <nhunjd$prv$2...@news.albasani.net>
>

that tries to open an e-mail, not a link...

[Deanna Earley]

It's actually an NNTP message ID.
But it points to your first reply in this thread where you give a
solution for some reason.

--
Deanna Earley (d...@earlsoft.co.uk, d...@doesnotcompute.co.uk)

(Replies direct to my email address will be printed, shredded then fed
to the dragons. Please reply to the group.)