Re: [PATCH v4 2/3] msvcr100: Implement _StructuredTaskCollection::_RunAndWait and _Schedule.

[Piotr Caban]

Hi Torge,

On 4/11/22 16:30, Torge Matthies wrote:
?_RunAndWait@_StructuredTaskCollection@details@Concurrency@@QAA?AW4_TaskCollectionStatus@23@PAV_UnrealizedChore@23@@Z
> @ stub -arch=i386 ?_RunAndWait@_StructuredTaskCollection@details@Concurrency@@QAG?AW4_TaskCollectionStatus@23@PAV_UnrealizedChore@23@@Z
> -@ stub -arch=win64 ?_RunAndWait@_StructuredTaskCollection@details@Concurrency@@QEAA?AW4_TaskCollectionStatus@23@PEAV_UnrealizedChore@23@@Z
> +@ thiscall -arch=win64

Please also add arm and i386 exports.
> +DEFINE_THISCALL_WRAPPER(StructuredTaskCollection__RunAndWait, 8)
> +/*enum Concurrency::details::_TaskCollectionStatus*/int __thiscall StructuredTaskCollection__RunAndWait(StructuredTaskCollection *this, UnrealizedChore *chore)
> +{
> - StructuredTaskCollectionChoresEntry *chores, *entry, *next;
> - LONG total_count = 0, created_count = 0;
> - TP_POOL *tpool = NULL;
> - TP_CALLBACK_ENVIRON cbenv;
> - volatile LONG waiting_on = 0;
> - int ret = 1;
> - LONG val;
> -
> - TRACE("(%p %p)\n", this, chore);
> -
> - chores = InterlockedExchangePointer((void *volatile *)&this->unk_chores, NULL);
> - this->count = 0;
> -
> - if (!chores)
> -
>    return ret;
> -
> - for (entry = chores; entry; entry = entry->next)
> -
>    total_count++;
> -
> - tpool = CreateThreadpool(NULL);
> - if (!tpool || !SetThreadpoolThreadMinimum(tpool, total_count))
> -
>    goto done;
> - SetThreadpoolThreadMaximum(tpool, total_count);
> -
> - memset(&cbenv, 0, sizeof(cbenv));
> - cbenv.Version = 1;
> - cbenv.Pool = tpool;
> -
> - for (entry = chores; entry; entry = entry->next)
> - {
> -
>    entry->waiting_on_ptr = &waiting_on;
> -
>    entry->work = CreateThreadpoolWork(StructuredTaskCollection_threadpool_cb, entry, &cbenv);
> -
>    if (!entry->work)
> -
>    {
> -
>        ret = 0;
> -
>        goto done;
> -
>    }
> -
>    created_count++;
> - }
> -
> - InterlockedExchange(&waiting_on, created_count);
> - for (entry = chores; entry; entry = entry->next)
> -
>    SubmitThreadpoolWork(entry->work);
> -
> - if (chore)
> -
>    StructuredTaskCollection_run_chore(chore, NULL);
> -
> - TRACE("waiting on %u workers\n", created_count);
> -
> - while ((val = InterlockedCompareExchange(&waiting_on, 0, 0)))
> -
>    RtlWaitOnAddress((void*)&waiting_on, (void*)&val, sizeof(waiting_on), NULL);
> -
>
> +done:
> - for (entry = chores; entry; entry = next)
> - {
> -
>    if (created_count > 0)
> -
>    {
> -
>        CloseThreadpoolWork(entry->work);
> -
>        created_count--;
> -
>    }
> -
>    next = entry->next;
> -
>    operator_delete(entry);
> - }
> -
> - if (tpool) CloseThreadpool(tpool);
> -
> - return ret;
>
> +}Please add tests - it's not really possible to implement it correctly

(or review the patch) without them. AFAICS the chores should be executed 
in LIFO order (waiting for previous task to complete).

Thanks,
Piotr