Re: [PATCH v2 2/3] msvcrt: Implement Concurrency::event

[Piotr Caban]

Hi Daniel,

I think it would be better to also avoid the static critical section. 
Here are some hints how it can be achieved:

typedef struct
{
     struct list *waiters;
     INT_PTR signaled;
     critical_section cs;
} event;

struct thread_wait;
typedef struct
{
     struct thread_wait *wait;
     struct list entry;
} thread_wait_entry;

typedef struct thread_wait
{
     event *signaled;
     int pending_waits;
     thread_wait_entry entries[1];
} thread_wait;
(thread_wait structure will need to be accessed in a thread safe way)

With such structures event_set may be implemented in following way (this 
describes the idea, it's not a fully correct code):
void __thiscall event_set(event *this)
{
     critical_section_lock(&this->cs);
     if (this->signaled) unlock and return;
     this->signaled = TRUE;

     LIST_FOR_EACH_ENTRY(cur, this->waiters) {
         if (!InterlockedDecrement(&cur->wait->pending_waits)) {
             if (!InterlockedCompareExchangePtr(&cur->wait->signaled, 
this, 0))
                 NtReleaseKeyedEvent(keyed_event, cur->wait, 0, NULL);
         }
     }
     critical_section_unlock(&this->cs);
}

void __thiscall event_reset(event *this)
{
     critical_section_lock(&this->cs);
     if (!this->signaled) unlock and return;
     this->signaled = FALSE;

     LIST_FOR_EACH_ENTRY(cur, this->waiters)
         InterlockedIncrement(&cur->wait->pending_waits);
     critical_section_unlock(&this->cs);
}

int __cdecl wait_for_multiple(event **events, MSVCRT_size_t count, 
MSVCRT_bool wait_all, unsigned int timeout)
{
     wait = allocate thread_wait struct;
     initialize wait struct;
     (wait->pending_waits = wait_all ? count : 1)

     for(int i=0; i<count; i++) {
         critical_section_lock(events[i]);
         add wait to waiters queue
         (decrease wait->pending_waits if necessary)
         critical_section_unlock(events[i]);
     }

     status = NtWaitForKeyedEvent(keyed_event, wait, ...);
     if(!status || InterlockedCompareExchangePtr(wait->signaled, 1, 0))
         get ret from wait->signaled
     else
         ret = COOPERATIVE_WAIT_TIMEOUT;

     for(int i=0; i<count; i++) {
         critical_section_lock(events[i]);
         remove wait from waiters queue
         critical_section_unlock(events[i]);
     }
}

Also I think it would be nice to avoid allocation in case of waiting for 
only one handle (or just in event_wait function).

What do you think about it?

Thank you,
Piotr

[PATCH v2 2/3] msvcrt: Implement Concurrency::event

[Daniel Lehman]

> remove wait from waiters queue
>          critical_section_unlock(events[i]);
>      }
> }
>
> Also I think it would be nice to avoid allocation in case of waiting for only one handle (or just in event_wait function).
>
> What do you think about it?

sounds good.  I'll update and resend