I believe WFMO is a core feature of using Win32-style events and most people will want to - at some point - wait on any/all of two or more events.

To that end, I propose changing the WFMO preprocessor define and #fdef gates to a NO_WFMO opposite/equivalent so that the pevents library is WFMO-enabled by default.

Thoughts and comments are welcome.

Since pevents is supposed to be a WFMO clone, I'm not sure if this is a bug report or a feature request, because the implementation just doesn't do this right now.

From the Remarks section of the documentation for WaitForMultipleObjects on MSDN (emphasis mine):

When bWaitAll is TRUE, the function's wait operation is completed only when the states of all objects have been set to signaled. The function does not modify the states of the specified objects until the states of all objects have been set to signaled. For example, a mutex can be signaled, but the thread does not get ownership until the states of the other objects are also set to signaled. In the meantime, some other thread may get ownership of the mutex, thereby setting its state to nonsignaled.

This means that during a WFME call where waitAll = true, the state of events shall not be reset, in the specific case of auto-reset Events, until all are simultaneously in the set state. This would apply to manual-reset Events too, except that their state cannot change due to a WaitForEvent/WFME call unless an explicit call to ResetEvent is made. Note that even though WMFE( waitAll = true, milliseconds = UINT64_MAX ) shall return only if all events are signaled, this bug applies to waitAll = true calls to WFME with any timeout value because the state shall only change once all events are in the set state, and the current implementation immediately resets the event before waiting if the Event is set.

This seems to be a significant change (hence, a feature request?) simply because the wait state is never added to any Event that is already in the set state, which is incorrect because another thread could call ResetEvent on any already-set Event, change the state back to not-set, and thus should increment the EventsLeft shared state member.

The following test case exhibits the error:

neosmart::neosmart_event_t lEvents[3];
lEvents[0] = neosmart::CreateEvent( false, true );  // Already Signaled AutoReset
lEvents[1] = neosmart::CreateEvent( false, false ); // Not Signaled AutoReset
lEvents[2] = neosmart::CreateEvent( false, true );  // Already Signaled AutoReset

// WFMO is non-destructive if a wait-all with any timeout value fails on auto-reset events.
if ( neosmart::WaitForMultipleEvents( lEvents, 3, TRUE, 0 ) == WAIT_OBJECT_0 )
    throw std::runtime_error( "Must not be signaled!" );

// FAILS!!
if ( neosmart::WaitForEvent( lEvents[0], 0 ) != WAIT_OBJECT_0 )
    throw std::runtime_error( "Must be signaled" );

if ( neosmart::WaitForEvent( lEvents[1], 0 ) != WAIT_TIMEOUT )
    throw std::runtime_error( "Must not be signaled" );

// FAILS!!
if ( neosmart::WaitForEvent( lEvents[2], 0 ) != WAIT_OBJECT_0 )
    throw std::runtime_error( "Must be signaled" );


// WFMO is destructive if a wait-all succeeds with any timeout value on auto-reset events.
for ( auto& lEvent : lEvents )
    neosmart::SetEvent( lEvent );
if ( neosmart::WaitForMultipleEvents( lEvents, 3, TRUE, 0 ) != WAIT_OBJECT_0 )  // OK
    throw std::runtime_error( "Must be signaled!" );
for ( auto& lEvent : lEvents )
{
    if ( neosmart::WaitForEvent( lEvent, 0 ) != WAIT_TIMEOUT ) // OK
        throw std::runtime_error( "Must not be signaled" );
    neosmart::DestroyEvent( lEvent );
}

The pthread_mutex_trylock implementation of WaitForEvent (with no timeout) is flawed as it can easily return WAIT_TIMEOUT when an Event, either auto-reset or manual-reset, is clearly in the signaled state. This issue only affects WaitForEvent calls where timeout = 0.

First, consider an already-signaled manual-reset event with 2 threads, A and B, each calling WaitForEvent( 0 ) concurrently in a tight loop. If A is holding the lock while B attempts to grab the lock, B will erroneously return WAIT_TIMEOUT, even though the event is always in the signaled state.

Since we know that:

1. The event is already signaled,
2. Manual-reset events can only change from Signaled -> Nonsignaled with a call to ResetEvent, and
3. Neither A nor B can change the signaled state of a manual-reset event solely by calling WaitForEvent

We know the pthread_mutex_trylock implementation is incorrect here.

Second, consider an already-signaled auto-reset event with several threads each calling SetEvent concurrently.
If some other thread, C, continuously loops on calls to WaitForEvent as follows:

using namespace neosmart;

// ...

auto event = CreateEvent( false, true ); // Already-set Auto-Reset Event

// Spawn some SetEvent threads...

bool signaled = true;
while ( signaled )
{
  signaled = WaitForEvent( event, 0 ) == 0;
  SetEvent( event );
}
// destroy, etc.

At some point, if one of the threads is calling SetEvent while C just begins a call to WaitForEvent, C's call to WaitForEvent will incorrectly return WAIT_TIMEOUT simply because C may fail to grab the lock.

Since we know that:

1. C is the only waiter,
2. There are no calls to ResetEvent by any threads, and
3. C ensures that upon every iteration of the loop, event is in the signaled state

C's call to WaitForEvent should never return WAIT_TIMEOUT in this case.

Tests cases exhibit this behavior perfectly. I'd put some up, but they're easy enough to reason about that it's not really necessary.

The only fix I see is to replace the pthread_mutex_trylock logic with a simple call to pthread_mutex_lock only.

I almost additionally recommended the possibility of making event->State atomic, and returning WAIT_TIMEOUT or 0 based on the value of the atomic load of event-State, except that auto-reset events must be reset before returning. This might warrant introducing extra logic just to figure out if you're able to reset the state. Since it's unclear what the ramifications are in that case, I think the only simply-correct solution is to grab the lock and block if its already taken.

I turn on the WFMO feature. Here is a function with issue.

bool RemoveExpiredWaitHelper(neosmart_wfmo_info_t_ wait)
{
    int result = pthread_mutex_trylock(&wait.Waiter->Mutex);  /* When result is 0, Mutex is locked. */

    if (result == EBUSY)
    {
        return false;
    }

    assert(result == 0);

    if (wait.Waiter->StillWaiting == false)
    {
        --wait.Waiter->RefCount;
        assert(wait.Waiter->RefCount >= 0);
        if (wait.Waiter->RefCount == 0)
        {
            wait.Waiter->Destroy();  /* Waiter's member Mutex is in locked state, we cannot destroy it. */
            delete wait.Waiter;
        }
        else
        {
            result = pthread_mutex_unlock(&wait.Waiter->Mutex);
            assert(result == 0);
        }

        return true;
    }

    result = pthread_mutex_unlock(&wait.Waiter->Mutex);
    assert(result == 0);

    return false;
}

i have tried pevents project on embedded linux.
before we call wait.Waiter->Destroy(),
wait.Waiter->Mutex is in locked state,
could this mutex object be destroyed successfully?

this embedded device has arm cpu,
runs linux kernel 3.10.31, pthread library 2.19,
i find in neosmart_wfmo_t_::Destroy(),
call to pthread_mutex_destroy failed, error number is EBUSY.

is this an issue, which leads to resource leak?

I'm experiencing deadlocks compiling the library under android, when using arm64-v8a ABI.
With armeabi-v7a everything looks fine.

While coming up with my own OO-implementation of Events based on pevents, noticed that pevents fails the following test case:

#include <stdexcept>
#include <vector>
void test()
{
        std::vector<neosmart::neosmart_event_t> lPEvents( 63 ); // Can be any number of events from 1-N.
        for ( auto& lEvent : lPEvents )
        {
            // Manual or Auto-Reset Events (doesn't matter which kind), and all already set.
            //
            // Note that this bug will manifest as long as all the events are
            // set before calling WaitForMultipleEvents.
            lEvent = neosmart::CreateEvent( false, true );
        }

        // Incorrectly returns non-signaled when checking state,
        // (i.e. timeout = 0, [check state and return immediately])
        auto lResult = neosmart::WaitForMultipleEvents( lPEvents.data(), 
                                                        static_cast<int>( lPEvents.size() ), 
                                                        true,// Bug only occurs with Wait All = True.
                                                        0 ); // Don't wait; check state and return immediately.

        // Cleanup first (in case we throw)
        for ( auto& lEvent : lPEvents )
        {
            neosmart::DestroyEvent( lEvent );
        }

        if ( lResult != 0 )
        {
            if ( lResult == WAIT_TIMEOUT ) // Spoilers:  It times out.
                throw std::runtime_error( "Events were already set, we WFME'd it, and it returned TIMEOUT!" );
            else throw std::runtime_error( "Events were already set, and WFME didn't return 0!" );
        }
}

Though WFME does still return immediately as specified, it should also return 0 in this case, denoting all events were signaled, but instead returns WAIT_TIMEOUT.

The bug involves failure to handle when all events have already been signaled when inside the event-check loop. The algorithm properly decrements the shared wait state toward zero, but then never recongizes the fact that when the state reaches 0, there is nothing remaining to do except report success.

The fix is simple: Insert the following conditional on line 269:

if (waitAll)
{
        --wfmo->Status.EventsLeft;
        assert(wfmo->Status.EventsLeft >= 0);
        if (wfmo->Status.EventsLeft == 0)
        {
                assert(i == (count - 1));
                done = true;
        }
}

Lost Wake-Up Problem Described On Oracle Website

A call to pthread_cond_signal() or pthread_cond_broadcast() when the thread does not hold the mutex lock associated with the condition can lead to lost wake-up bugs.

int SetEvent(neosmart_event_t event)
{
    int result = pthread_mutex_lock(&event->Mutex);
    /* .... */
    result = pthread_mutex_unlock(&event->Mutex);

    result = pthread_cond_signal(&event->CVariable);  // the thread does not hold the mutex lock.
    /* .... */
    return 0;
}

Hello,
I'm a former C/C++ Windows NT only programmer (1993 until 2007) and now I support both Windows and Posix

So your library is very helpful to port my code to posix multihread. Thank you. It's a very clean and professional code.

I suggest you create a "1.0" tag on github (and update sometime). This help inclusion on professional package

Thank you !

Use an intrusive list to keep track of outstanding waits instead of pushing WFMO callers to a per-event std::vector. This will reduce the dependence on the C++ standard library (and might make the code C-compatible out-of-the-box), reduce allocations, and improve performance.

Hi team,

I was wondering if there is any plan to support WaitForMultipleThreads just like calling WaitForMultipleObjects on threads in windows?

Thanks.
Wenbo

when WaitForEvent is not in a waiting state, using setevent should return failure