[SYCL][E2E] Destroy the Level Zero handles interop-direct.cpp owns - #23123
Open
uditagarwal97 wants to merge 1 commit into
Open
[SYCL][E2E] Destroy the Level Zero handles interop-direct.cpp owns#23123uditagarwal97 wants to merge 1 commit into
uditagarwal97 wants to merge 1 commit into
Conversation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The RAII lifetime ordering correctly prevents leaks without invalidating live SYCL interop objects.
Pull request overview
Adds RAII cleanup for Level Zero handles retained with ownership::keep.
Changes:
- Tracks context, command queue, and command list handles.
- Destroys handles safely after dependent SYCL objects, including early-return paths.
File summaries
| File | Description |
|---|---|
sycl/test-e2e/Adapters/level_zero/interop-direct.cpp |
Adds ordered cleanup for test-owned Level Zero resources. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
uditagarwal97
marked this pull request as ready for review
September 8, 2026 23:26
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
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.
Bug
Adapters/level_zero/interop-direct.cppcreates aze_context, aze_command_queueand an immediateze_command_listdirectly through the Level Zero API and never destroys any of them. All three are handed to SYCL withext::oneapi::level_zero::ownership::keep, so destroying them is the test's own responsibility.Under an ASan+UBSan runtime build the test fails with 20768 bytes leaked in 14 allocations (7 per
RUNline, and there are two). Every frame is in the test's ownmainplus the vendor Level Zero driver — there is no SYCL or Unified Runtime frame anywhere in the report, so this is purely the test leaking:Fix
Destroy the three handles the test owns.
Ordering matters:
InteropContext,InteropQueueCQandInteropQueueCLare function-scope SYCL objects, so their destructors run after the closingreturn. CallingzeContextDestroyjust beforereturn 0would therefore tear the context down while SYCL objects still referencing it are alive. A small RAII holder declared before the SYCL interop objects is destroyed last, which gives the right order and also covers the eleven earlyreturn 1paths without reindenting the body.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com