Skip to content
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

Add umfInit/umfTearDown to urAdapterGet/urAdapterRelease #2007

Conversation

omarahmed1111
Copy link
Contributor

@omarahmed1111 omarahmed1111 commented Aug 23, 2024

As windows order of unloading dlls is reversed from linux, windows will call umfTearDown before it could release umf objects in level_zero, so we call umfInit on urAdapterGet and umfAdapterTearDown to enforce the teardown of umf
after umf objects are destructed.

Intel/llvm PR

@omarahmed1111 omarahmed1111 requested a review from a team as a code owner August 23, 2024 09:56
@github-actions github-actions bot added the level-zero L0 adapter specific issues label Aug 23, 2024
@omarahmed1111 omarahmed1111 added the v0.10.x Include in the v0.10.x release label Aug 23, 2024
@omarahmed1111 omarahmed1111 requested a review from a team as a code owner August 27, 2024 09:56
@github-actions github-actions bot added the common Changes or additions to common utilities label Aug 27, 2024
@omarahmed1111 omarahmed1111 force-pushed the add-umfInit/umfTearDown-to-urAdapterGet/urAdapterRelease branch 2 times, most recently from c2fd168 to 9e92cf8 Compare August 27, 2024 13:39
Comment on lines +30 to +35
// This is a temporary workaround on windows, where UR adapter is teardowned
// before the UR loader, which will result in access violation when we use print
// function as the overrided print function was already released with the UR
// adapter.
// TODO: Change adapters to use a common sink class in the loader instead of
// using thier own sink class that inherit from logger::Sink.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not ideal, but it should work for now to solve a release problem regarding the logger. @igchor @nrspruit If this is okay, I will merge this asap.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think that's fine for now. Eventually, we could do a similar thing like here: #2000. Just keep a global instance of shared_ptr<> to the logger (here

) and every time get_logger is called, we would return shared_ptr instead of a reference.

@omarahmed1111 omarahmed1111 force-pushed the add-umfInit/umfTearDown-to-urAdapterGet/urAdapterRelease branch from 9e92cf8 to 581a0d8 Compare August 27, 2024 14:24
@igchor igchor merged commit 05d3ea7 into oneapi-src:main Aug 27, 2024
74 of 75 checks passed
omarahmed1111 pushed a commit to omarahmed1111/unified-runtime that referenced this pull request Sep 10, 2024
…TearDown-to-urAdapterGet/urAdapterRelease

Add umfInit/umfTearDown to urAdapterGet/urAdapterRelease
@kbenzie kbenzie removed the v0.10.x Include in the v0.10.x release label Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common Changes or additions to common utilities level-zero L0 adapter specific issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants