[SYCL][UR][L0] Fix urTearDown multiple call crash #10512
Conversation
| for (ur_platform_handle_t Platform : *PiPlatformsCache) { | ||
| delete Platform; | ||
|
|
||
| if (PiPlatformsCache) { |
There was a problem hiding this comment.
thanks @bratpiorka
@callumfare : does this conflict with your PR in #10349 ? would it be safe to merge this and then to rebase yours on top of it?
There was a problem hiding this comment.
This will conflict but it should be a simple fix, I can rebase my PR when this is merged
There was a problem hiding this comment.
I don't think this patch will be necessary after your changes @callumfare since urTeardown will be an empty function and the actual teardown code (what this patch is changing) will be protected with a reference count.
There was a problem hiding this comment.
That's a good point, the case of multiple teardowns won't be a problem any more.
One related scenario I've just thought of is when the adapter is fully released and the reference count is 0, but is then reinitialized with a call to urAdapterGet again. The teardown should leave the adapter in a state where it's safe to call the setup code again. I'm not sure if this PR will ensure that but maybe that's something to consider in a future change.
There was a problem hiding this comment.
thanks @callumfare , @pbalcer . So, do we want to not merge this patch in favor of yours?
There was a problem hiding this comment.
Yes, that's my recommendation.
@callumfare We should definitely have wording to that effect in the docs for related functions, with a matching CTS scenario.
There was a problem hiding this comment.
Ok, so I'll close this PR, thanks for the clarification
4 participants
This fix prevents from the crash when urTearDown is called multiple time