Re: [PATCH] msvcr120: Implemented Concurrency::details::_Condition_variable.

[Piotr Caban]

Hi,

On 02/14/15 01:52, Yifu Wang wrote:
> Implementation of Concurrency::details::_Condition_variable along with a producer-consumer test case.
> The class is used in msvcp120.dll by _Cnd_t functions, which are used in std::condition_variable and std::thread.
> ---
>   dlls/msvcr120/msvcr120.spec    |  36 +++---
>   dlls/msvcr120/tests/msvcr120.c | 277 +++++++++++++++++++++++++++++++++++++++++
>   dlls/msvcrt/lock.c             | 171 +++++++++++++++++++++++++
>   3 files changed, 466 insertions(+), 18 deletions(-)

Please also update msvcr110.spec and msvcr120_app.spec files.

+@ thiscall -arch=i386 
?wait_for(a)_Condition_variable@details(a)Concurrency@@QAE_NAAVcritical_section(a)3@I(a)Z(ptr 
ptr ptr) _Condition_variable_wait_for
The arguments types in spec file are incorrect (in this case there 
should be: ptr ptr long). I was not checking other entries.

/* signal */
produced = consumed = 0;
for(i = 0; i < NUM_PRODUCER; ++ i)
     t[i] = CreateThread(0, 0, producer_thread, (LPVOID)0, 0, 0);
for(i = NUM_PRODUCER; i < NUM_PRODUCER + NUM_CONSUMER; ++ i)
     t[i] = CreateThread(0, 0, consumer_thread, (LPVOID)0, 0, 0);

You don't need the (LPVOID)0 casts here.

/* a quick sanity test on timed wait */
for(i = 0; i < 200; i += 10) {
     call_func1(p_critical_section_lock, &buf_cs);
     wake = (MSVCRT_bool)call_func3(p__Condition_variable_wait_for, 
&buf_not_empty, &buf_cs, i);
     ok(wake == FALSE, "_Condition_variable not supposed to wake\n");
     call_func1(p_critical_section_unlock, &buf_cs);
}

I don't think that spending ~2 seconds on this tests is useful. It's 
enough to test it once (not in a loop).

Cheers,
Piotr

[PATCH] msvcr120: Implemented Concurrency::details::_Condition_variable.

[Piotr Caban]

Hi,

there's also a leak of cv_queue structure in 
_Condition_variable_wait_for. It's never freed when 
_Condition_varaible_notify* is not called later.

Cheers,
Piotr