Skip to content

[SYCL] Fix unloading of shared images on Windows #19074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

steffenlarsen
Copy link
Contributor

After #17869, Windows will not unload images during the runtime of the program. However, this means that shared libraries that are dynamically loaded and unloaded will leave dangling pointers to their images. If somehow these images get picked up, e.g. if another library is loaded with overlapping symbols, the runtime may try to dereference the dangling device image pointers. This commit reenables Windows unloading with the caveat that once the global handler is dead the runtime assumes that shutdown has begun.

@steffenlarsen
Copy link
Contributor Author

@cperkinsintel & @nrspruit - I think we need to reopen this can of worms. Though it is not immediately obvious, this is actually affecting sycl/test-e2e/syclcompat/kernel/kernel_win.cpp and sycl/unittests/Extensions/RootGroup.cpp. They started failing for the changes in #18949 and I narrowed the failures down to the cause described in this PR.

After intel#17869, Windows will not unload
images during the runtime of the program. However, this means that
shared libraries that are dynamically loaded and unloaded will leave
dangling pointers to their images. If somehow these images get picked
up, e.g. if another library is loaded with overlapping symbols, the
runtime may try to dereference the dangling device image pointers. This
commit reenables Windows unloading with the caveat that once the global
handler is dead the runtime assumes that shutdown has begun.

Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen steffenlarsen force-pushed the steffen/reenable_shutdown_cleanup_windows branch from 04ab3fb to c79dd80 Compare June 19, 2025 12:56
steffenlarsen pushed a commit that referenced this pull request Jun 23, 2025
Changes the `MockDeviceImage` and `MockDeviceImageArray` to static
variables so that they're not created in each test case. Relates to
#19068 and #19074. Fixes #18892.

Signed-off-by: Michael Aziz <[email protected]>
@steffenlarsen steffenlarsen marked this pull request as ready for review June 23, 2025 07:15
@steffenlarsen steffenlarsen requested a review from a team as a code owner June 23, 2025 07:15
@steffenlarsen
Copy link
Contributor Author

Talking to @cperkinsintel offline, it turns out this is never called on Windows, so this will have no effect.

@steffenlarsen steffenlarsen deleted the steffen/reenable_shutdown_cleanup_windows branch June 24, 2025 13:59
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.

1 participant