[PATCH 0/1] MR889: msvcr100: Fix crash when setting scheduler in schedule_task_proc.

[Piotr Caban (＠piotr)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53672

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/889

[PATCH 1/1] msvcr100: Fix crash when setting scheduler in schedule_task_proc.

[Piotr Caban]

From: Piotr Caban <piotr(a)codeweavers.com>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53672
---
 dlls/msvcrt/concurrency.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/msvcrt/concurrency.c b/dlls/msvcrt/concurrency.c
index a1c5c635602..f7f7adb86ed 100644
--- a/dlls/msvcrt/concurrency.c
+++ b/dlls/msvcrt/concurrency.c
@@ -1342,7 +1342,7 @@ static void WINAPI schedule_task_proc(PTP_CALLBACK_INSTANCE instance, void *cont
     arg = *(schedule_task_arg*)context;
     operator_delete(context);
 
-    if(&arg.scheduler->scheduler != try_get_current_scheduler()) {
+    if(&arg.scheduler->scheduler != get_current_scheduler()) {
         ThreadScheduler_Attach(arg.scheduler);
         detach = TRUE;
     }
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/889

[Torge Matthies (＠tmatthies)]

Ye I found this one too, was planning on submitting a fix but didn't get to it yet (and didn't know there was already a bug report about it).

Something like this should also work and avoids creating a Context, choose which one is better:
```diff
diff --git a/dlls/msvcrt/concurrency.c b/dlls/msvcrt/concurrency.c
index fe73c0fdd400..d483cfd39d6e 100644
--- a/dlls/msvcrt/concurrency.c
+++ b/dlls/msvcrt/concurrency.c
@@ -1337,12 +1337,14 @@ void __cdecl CurrentScheduler_Detach(void);
 static void WINAPI schedule_task_proc(PTP_CALLBACK_INSTANCE instance, void *context, PTP_WORK work)
 {
     schedule_task_arg arg;
+    Scheduler *curr_scheduler;
     BOOL detach = FALSE;
 
     arg = *(schedule_task_arg*)context;
     operator_delete(context);
 
-    if(&arg.scheduler->scheduler != try_get_current_scheduler()) {
+    curr_scheduler = try_get_current_scheduler();
+    if(&arg.scheduler->scheduler != (curr_scheduler ? curr_scheduler : &default_scheduler->scheduler)) {
         ThreadScheduler_Attach(arg.scheduler);
         detach = TRUE;
     }
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/889#note_8809

[Piotr Caban (＠piotr)]

On Mon Sep 19 11:45:14 2022 +0000, Torge Matthies wrote:
> Ye I found this one too, was planning on submitting a fix but didn't get
> to it yet (and didn't know there was already a bug report about it).
> Something like this should also work and avoids creating a Context,
> choose which one is better:
> ```diff
> diff --git a/dlls/msvcrt/concurrency.c b/dlls/msvcrt/concurrency.c
> index fe73c0fdd400..d483cfd39d6e 100644
> --- a/dlls/msvcrt/concurrency.c
> +++ b/dlls/msvcrt/concurrency.c
> @@ -1337,12 +1337,14 @@ void __cdecl CurrentScheduler_Detach(void);
>  static void WINAPI schedule_task_proc(PTP_CALLBACK_INSTANCE instance,
> void *context, PTP_WORK work)
>  {
>      schedule_task_arg arg;
> +    Scheduler *curr_scheduler;
>      BOOL detach = FALSE;
>
> arg = *(schedule_task_arg*)context;
>      operator_delete(context);
>
> -    if(&arg.scheduler->scheduler != try_get_current_scheduler()) {
> +    curr_scheduler = try_get_current_scheduler();
> +    if(&arg.scheduler->scheduler != (curr_scheduler ? curr_scheduler :
> &default_scheduler->scheduler)) {
>          ThreadScheduler_Attach(arg.scheduler);
>          detach = TRUE;
>      }
> ```
> The tasks created with `ScheduleTask()` should have the context created so there's no point in trying to avoid it.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/889#note_8811