Skip to content

[SYCL] Do not release threadpool resources on Windows during shutdown - #23067

Merged
uditagarwal97 merged 2 commits into
syclfrom
private/udit/win_host_tasks
Sep 4, 2026
Merged

[SYCL] Do not release threadpool resources on Windows during shutdown#23067
uditagarwal97 merged 2 commits into
syclfrom
private/udit/win_host_tasks

Conversation

@uditagarwal97

@uditagarwal97 uditagarwal97 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Fixes flaky crash when joining host tasks during windows shutdown.

Windows does not guarantee that threads of main application won't be killed unless DllMain(detach) is complete:

When handling DLL_PROCESS_DETACH, a DLL should free resources such as heap memory only if the DLL is being unloaded dynamically (the lpvReserved parameter is NULL). If the process is terminating (the lpvReserved parameter is non-NULL), all threads in the process except the current thread either have exited already or have been explicitly terminated by a call to the ExitProcess function, which might leave some process resources such as heaps in an inconsistent state. In this case, it is not safe for the DLL to clean up the resources. Instead, the DLL should allow the operating system to reclaim the memory.

DllMain entry point (Process.h) - Win32 apps | Microsoft Learn

So, we should not try to join host tasks on windows during shutdown.

Fixes CMPLRLLVM-74436

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts SYCL runtime shutdown behavior to avoid releasing certain global/threadpool resources on Windows, aiming to prevent shutdown-time races/crashes when host tasks are in flight.

Changes:

  • Modifies shutdown_early() threadpool cleanup logic with platform-specific behavior.
  • Prevents deleting the GlobalHandler singleton on Windows in shutdown_late() (intended leak for shutdown safety).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread sycl/source/detail/global_handler.cpp Outdated
Comment thread sycl/source/detail/global_handler.cpp

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

sycl/source/detail/global_handler.cpp:389

  • Skipping delete GlobalHandler::RTGlobalObjHandler on Windows leaks the GlobalHandler for all teardown paths, including the documented FreeLibrary unload case (where the process keeps running and the OS will not reclaim the memory). If the intent is to avoid teardown only during process termination, consider keying this decision off the Windows detach reason (e.g., persist lpReserved != NULL/process-termination state from shutdown_early and only skip delete in that case).
#ifndef _WIN32
  // Release the rest of global resources. Do not release GlobalHandler
  // on Windows and let OS reclaim leaked memory. Releasing GlobalHandler
  // on Windows can seg fault if application uses host tasks, as there
  // can be a race between host tasks and shutdown.

Comment thread sycl/source/detail/global_handler.cpp
@uditagarwal97
uditagarwal97 marked this pull request as ready for review September 3, 2026 04:08
@uditagarwal97
uditagarwal97 requested a review from a team as a code owner September 3, 2026 04:08
@uditagarwal97
uditagarwal97 merged commit 39c9a97 into sycl Sep 4, 2026
31 of 32 checks passed
@uditagarwal97
uditagarwal97 deleted the private/udit/win_host_tasks branch September 4, 2026 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants