[SYCL] Avoid releasing UR objects on sycl destruction on windows #15182
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
omarahmed1111
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
omarahmed1111
force-pushed
the
release-default-context-on-windows-before-unloading-adapters
branch
2 times, most recently
from
65a9843 to
bb59a3f
Compare
omarahmed1111
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
omarahmed1111
force-pushed
the
release-default-context-on-windows-before-unloading-adapters
branch
2 times, most recently
from
840c74e to
c711d7b
Compare
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
omarahmed1111
force-pushed
the
release-default-context-on-windows-before-unloading-adapters
branch
from
c711d7b to
030abe2
Compare
omarahmed1111
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
omarahmed1111
force-pushed
the
release-default-context-on-windows-before-unloading-adapters
branch
from
030abe2 to
2e6d997
Compare
omarahmed1111
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
omarahmed1111
force-pushed
the
release-default-context-on-windows-before-unloading-adapters
branch
from
2e6d997 to
7423b42
Compare
omarahmed1111
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
omarahmed1111
force-pushed
the
release-default-context-on-windows-before-unloading-adapters
branch
from
7423b42 to
8b20f94
Compare
omarahmed1111
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
kbenzie
changed the title
omarahmed1111
force-pushed
the
release-default-context-on-windows-before-unloading-adapters
branch
from
8b20f94 to
429bb3d
Compare
omarahmed1111
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
omarahmed1111
force-pushed
the
release-default-context-on-windows-before-unloading-adapters
branch
from
429bb3d to
639b76e
Compare
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
omarahmed1111
force-pushed
the
release-default-context-on-windows-before-unloading-adapters
branch
from
639b76e to
f6468ca
Compare
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
AllanZyne
reviewed
Sep 3, 2024
victor-eds
reviewed
Sep 3, 2024
omarahmed1111
force-pushed
the
release-default-context-on-windows-before-unloading-adapters
branch
from
c3e7b46 to
565fb13
Compare
omarahmed1111
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
omarahmed1111
force-pushed
the
release-default-context-on-windows-before-unloading-adapters
branch
from
565fb13 to
436ec4f
Compare
omarahmed1111
force-pushed
the
release-default-context-on-windows-before-unloading-adapters
branch
from
436ec4f to
498c7b6
Compare
omarahmed1111
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
omarahmed1111
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
omarahmed1111
changed the title
pbalcer
reviewed
Sep 6, 2024
| #ifdef _WIN32 | ||
| if (!sycl::detail::GlobalHandler::instance().isUrTearDowned) { | ||
| const PluginPtr &Plugin = getSyclObjImpl(MContext)->getPlugin(); | ||
| Plugin->call(urProgramRelease, MProgram); |
There was a problem hiding this comment.
would it be possible for the call function to internally check isUrTearDowned and return some 'unitinitalized' value (or just ignore it)? That way, we generically workaround this issue in one place, rather than doing in on a case-by-case basis.
|
This would be redundant as a mechanism already exist for tracking URAdapterRelease call |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SYCL-RT depends currently on the call to
DLLMainto begin its destruction on windows. However, when sycl-rtDLLMainis called, UR have already began its destruction on another thread and in case of level_zero, level_zero_loader.dll also began its destruction far before that, this would lead to race condition and access violations. as when release of UR objects begin either level_zero loader/UR adapter/UR loader have been unloaded already.So, this PR is a guard for that to not call UR objects release when sycl-rt began its destruction.