[PATCH 0/5] MR3023: msvcr100: Use Context::Block() in more places + bug fixes
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msvcp90/msvcp90.h | 2 +- dlls/msvcrt/concurrency.c | 64 ++++++++++++++++++++++++--------------- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/dlls/msvcp90/msvcp90.h b/dlls/msvcp90/msvcp90.h index 5b77a8164e8..c9e8e05566c 100644 --- a/dlls/msvcp90/msvcp90.h +++ b/dlls/msvcp90/msvcp90.h @@ -47,7 +47,7 @@ typedef struct cs_queue { void *ctx; struct cs_queue *next; - LONG status; + LONG free; int unknown; } cs_queue; diff --git a/dlls/msvcrt/concurrency.c b/dlls/msvcrt/concurrency.c index 66adef1fa6d..190d1f31c31 100644 --- a/dlls/msvcrt/concurrency.c +++ b/dlls/msvcrt/concurrency.c @@ -214,14 +214,12 @@ struct scheduled_chore { }; /* keep in sync with msvcp90/msvcp90.h */ -#define CS_UNLOCK 1 -#define CS_TIMEOUT 2 typedef struct cs_queue { Context *ctx; struct cs_queue *next; #if _MSVCR_VER >= 110 - LONG status; + LONG free; int unknown; #endif } cs_queue; @@ -2533,7 +2531,7 @@ void __thiscall critical_section_unlock(critical_section *this) while(1) { cs_queue *next; - if(!InterlockedCompareExchange(&this->unk_active.next->status, CS_UNLOCK, 0)) + if(!InterlockedExchange(&this->unk_active.next->free, TRUE)) break; next = this->unk_active.next; @@ -2572,12 +2570,40 @@ static void set_timeout(FILETIME *ft, unsigned int timeout) } #if _MSVCR_VER >= 110 +struct timeout_unlock +{ + Context *ctx; + BOOL timed_out; +}; + static void WINAPI timeout_unlock(TP_CALLBACK_INSTANCE *instance, void *ctx, TP_TIMER *timer) { - cs_queue *q = ctx; + struct timeout_unlock *tu = ctx; + tu->timed_out = TRUE; + call_Context_Unblock(tu->ctx); +} + +/* returns TRUE if wait has timed out */ +static BOOL block_context_for(Context *ctx, unsigned int timeout) +{ + struct timeout_unlock tu = { ctx }; + TP_TIMER *tp_timer; + FILETIME ft; + + tp_timer = CreateThreadpoolTimer(timeout_unlock, &tu, NULL); + if(!tp_timer) { + FIXME("throw exception?\n"); + return TRUE; + } + set_timeout(&ft, timeout); + SetThreadpoolTimer(tp_timer, &ft, 0, 0); + + call_Context_Block(ctx); - if(!InterlockedCompareExchange(&q->status, CS_TIMEOUT, 0)) - call_Context_Unblock(q->ctx); + SetThreadpoolTimer(tp_timer, NULL, 0, 0); + WaitForThreadpoolTimerCallbacks(tp_timer, TRUE); + CloseThreadpoolTimer(tp_timer); + return tu.timed_out; } /* ?try_lock_for(a)critical_section@Concurrency@@QAE_NI(a)Z */ @@ -2603,27 +2629,15 @@ bool __thiscall critical_section_try_lock_for( last = InterlockedExchangePointer(&this->tail, q); if(last) { - TP_TIMER *tp_timer; - FILETIME ft; - last->next = q; - tp_timer = CreateThreadpoolTimer(timeout_unlock, q, NULL); - if(!tp_timer) { - FIXME("throw exception?\n"); - return FALSE; + if(block_context_for(q->ctx, timeout)) + { + if(!InterlockedExchange(&q->free, TRUE)) + return FALSE; + /* Context was unblocked because of timeout and unlock operation */ + call_Context_Block(ctx); } - set_timeout(&ft, timeout); - SetThreadpoolTimer(tp_timer, &ft, 0, 0); - - call_Context_Block(q->ctx); - - SetThreadpoolTimer(tp_timer, NULL, 0, 0); - WaitForThreadpoolTimerCallbacks(tp_timer, TRUE); - CloseThreadpoolTimer(tp_timer); - - if(q->status == CS_TIMEOUT) - return FALSE; } cs_set_head(this, q); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3023
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msvcrt/concurrency.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dlls/msvcrt/concurrency.c b/dlls/msvcrt/concurrency.c index 190d1f31c31..560159ee00d 100644 --- a/dlls/msvcrt/concurrency.c +++ b/dlls/msvcrt/concurrency.c @@ -2590,6 +2590,11 @@ static BOOL block_context_for(Context *ctx, unsigned int timeout) TP_TIMER *tp_timer; FILETIME ft; + if(timeout == COOPERATIVE_TIMEOUT_INFINITE) { + call_Context_Block(ctx); + return FALSE; + } + tp_timer = CreateThreadpoolTimer(timeout_unlock, &tu, NULL); if(!tp_timer) { FIXME("throw exception?\n"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3023
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msvcrt/concurrency.c | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/dlls/msvcrt/concurrency.c b/dlls/msvcrt/concurrency.c index 560159ee00d..aed911a52a3 100644 --- a/dlls/msvcrt/concurrency.c +++ b/dlls/msvcrt/concurrency.c @@ -2569,7 +2569,6 @@ static void set_timeout(FILETIME *ft, unsigned int timeout) ft->dwLowDateTime = to.QuadPart; } -#if _MSVCR_VER >= 110 struct timeout_unlock { Context *ctx; @@ -2611,6 +2610,7 @@ static BOOL block_context_for(Context *ctx, unsigned int timeout) return tu.timed_out; } +#if _MSVCR_VER >= 110 /* ?try_lock_for(a)critical_section@Concurrency@@QAE_NI(a)Z */ /* ?try_lock_for(a)critical_section@Concurrency@@QEAA_NI(a)Z */ DEFINE_THISCALL_WRAPPER(critical_section_try_lock_for, 8) @@ -3478,33 +3478,11 @@ bool __thiscall _ReentrantBlockingLock__TryAcquire(_ReentrantBlockingLock *this) return TryEnterCriticalSection(&this->cs); } -static void WINAPI wait_timeout(TP_CALLBACK_INSTANCE *instance, void *arg, TP_TIMER *timer) -{ - Context *ctx = arg; - - call_Context_Unblock(ctx); -} - /* ?wait(a)Concurrency@@YAXI(a)Z */ void __cdecl Concurrency_wait(unsigned int time) { - Context *ctx = get_current_context(); - TP_TIMER *tp_timer; - FILETIME ft; - TRACE("(%d)\n", time); - - tp_timer = CreateThreadpoolTimer(wait_timeout, ctx, NULL); - if(!tp_timer) { - FIXME("throw exception?\n"); - Sleep(time); - return; - } - set_timeout(&ft, time); - SetThreadpoolTimer(tp_timer, &ft, 0, 0); - - call_Context_Block(ctx); - CloseThreadpoolTimer(tp_timer); + block_context_for(get_current_context(), time); } #if _MSVCR_VER>=110 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3023
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msvcrt/concurrency.c | 33 +++------------------------------ 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/dlls/msvcrt/concurrency.c b/dlls/msvcrt/concurrency.c index aed911a52a3..b96e0cbda6f 100644 --- a/dlls/msvcrt/concurrency.c +++ b/dlls/msvcrt/concurrency.c @@ -2850,17 +2850,8 @@ static inline int evt_transition(void **state, void *from, void *to) return InterlockedCompareExchangePointer(state, to, from) == from; } -static void WINAPI evt_timeout(TP_CALLBACK_INSTANCE *instance, void *ctx, TP_TIMER *timer) -{ - thread_wait *wait = ctx; - - if(evt_transition(&wait->signaled, EVT_WAITING, EVT_RUNNING)) - call_Context_Unblock(wait->ctx); -} - static size_t evt_wait(thread_wait *wait, event **events, int count, bool wait_all, unsigned int timeout) { - TP_TIMER *tp_timer = NULL; int i; wait->signaled = EVT_RUNNING; @@ -2884,30 +2875,12 @@ static size_t evt_wait(thread_wait *wait, event **events, int count, bool wait_a if(!timeout) return evt_end_wait(wait, events, count); - if (timeout != COOPERATIVE_TIMEOUT_INFINITE) - { - FILETIME ft; - - tp_timer = CreateThreadpoolTimer(evt_timeout, wait, NULL); - if(!tp_timer) { - FIXME("throw exception?\n"); - return COOPERATIVE_WAIT_TIMEOUT; - } - set_timeout(&ft, timeout); - SetThreadpoolTimer(tp_timer, &ft, 0, 0); - } - if(!evt_transition(&wait->signaled, EVT_RUNNING, EVT_WAITING)) return evt_end_wait(wait, events, count); - call_Context_Block(wait->ctx); - - if (tp_timer) - { - SetThreadpoolTimer(tp_timer, NULL, 0, 0); - WaitForThreadpoolTimerCallbacks(tp_timer, TRUE); - CloseThreadpoolTimer(tp_timer); - } + if(block_context_for(wait->ctx, timeout) && + !evt_transition(&wait->signaled, EVT_WAITING, EVT_RUNNING)) + call_Context_Block(wait->ctx); return evt_end_wait(wait, events, count); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3023
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msvcrt/concurrency.c | 39 +++++++++++++++------------------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/dlls/msvcrt/concurrency.c b/dlls/msvcrt/concurrency.c index b96e0cbda6f..eb9e8f9ee32 100644 --- a/dlls/msvcrt/concurrency.c +++ b/dlls/msvcrt/concurrency.c @@ -313,6 +313,7 @@ typedef struct #if _MSVCR_VER >= 110 #define CV_WAKE (void*)1 typedef struct cv_queue { + Context *ctx; struct cv_queue *next; LONG expired; } cv_queue; @@ -3035,20 +3036,19 @@ void __thiscall _Condition_variable_dtor(_Condition_variable *this) DEFINE_THISCALL_WRAPPER(_Condition_variable_wait, 8) void __thiscall _Condition_variable_wait(_Condition_variable *this, critical_section *cs) { - cv_queue q, *next; + cv_queue q; TRACE("(%p, %p)\n", this, cs); + q.ctx = get_current_context(); + q.expired = FALSE; critical_section_lock(&this->lock); q.next = this->queue; - q.expired = FALSE; - next = q.next; this->queue = &q; critical_section_unlock(&this->lock); critical_section_unlock(cs); - while (q.next != CV_WAKE) - RtlWaitOnAddress(&q.next, &next, sizeof(next), NULL); + call_Context_Block(q.ctx); critical_section_lock(cs); } @@ -3058,35 +3058,26 @@ DEFINE_THISCALL_WRAPPER(_Condition_variable_wait_for, 12) bool __thiscall _Condition_variable_wait_for(_Condition_variable *this, critical_section *cs, unsigned int timeout) { - LARGE_INTEGER to; - NTSTATUS status; - FILETIME ft; - cv_queue *q, *next; + cv_queue *q; TRACE("(%p %p %d)\n", this, cs, timeout); q = operator_new(sizeof(cv_queue)); + q->ctx = get_current_context(); + q->expired = FALSE; critical_section_lock(&this->lock); q->next = this->queue; - q->expired = FALSE; - next = q->next; this->queue = q; critical_section_unlock(&this->lock); critical_section_unlock(cs); - GetSystemTimeAsFileTime(&ft); - to.QuadPart = ((LONGLONG)ft.dwHighDateTime << 32) + - ft.dwLowDateTime + (LONGLONG)timeout * TICKSPERMSEC; - while (q->next != CV_WAKE) { - status = RtlWaitOnAddress(&q->next, &next, sizeof(next), &to); - if(status == STATUS_TIMEOUT) { - if(!InterlockedExchange(&q->expired, TRUE)) { - critical_section_lock(cs); - return FALSE; - } - break; + if(block_context_for(q->ctx, timeout)) { + if(!InterlockedExchange(&q->expired, TRUE)) { + critical_section_lock(cs); + return FALSE; } + call_Context_Block(q->ctx); } operator_delete(q); @@ -3118,7 +3109,7 @@ void __thiscall _Condition_variable_notify_one(_Condition_variable *this) node->next = CV_WAKE; if(!InterlockedExchange(&node->expired, TRUE)) { - RtlWakeAddressSingle(&node->next); + call_Context_Unblock(node->ctx); return; } else { operator_delete(node); @@ -3148,7 +3139,7 @@ void __thiscall _Condition_variable_notify_all(_Condition_variable *this) ptr->next = CV_WAKE; if(!InterlockedExchange(&ptr->expired, TRUE)) - RtlWakeAddressSingle(&ptr->next); + call_Context_Unblock(ptr->ctx); else operator_delete(ptr); ptr = next; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3023
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=133589 Your paranoid android. === debian11 (32 bit report) === quartz: dsoundrender: Timeout
participants (3)
-
Marvin -
Piotr Caban -
Piotr Caban (＠piotr)