Skip to content

[UR][L0 v1/v2 adapter]Handle errors of img functions #18261

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

Merged
merged 3 commits into from
May 8, 2025

Conversation

mateuszpn
Copy link
Contributor

@mateuszpn mateuszpn commented Apr 30, 2025

Handle errors in image functions to avoid memory leaks

@@ -274,7 +274,15 @@ ur_result_t createUrImgFromZeImage(ze_context_handle_t hContext,
ur_exp_image_mem_native_handle_t *pImg) {
ze_image_handle_t ZeImage;
ZE2UR_CALL(zeImageCreate, (hContext, hDevice, &ZeImageDesc, &ZeImage));
ZE2UR_CALL(zeContextMakeImageResident, (hContext, hDevice, ZeImage));
if (ZeImage == nullptr) {
Copy link
Member

Choose a reason for hiding this comment

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

Is it actually possible for ZeImage to be nullptr if we got SUCCESS from zeImageCreate?

try {
ZE2UR_CALL_THROWS(zeContextMakeImageResident, (hContext, hDevice, ZeImage));
} catch (const ze_result_t &result) {
ZE2UR_CALL(zeImageDestroy, (ZeImage));
Copy link
Member

Choose a reason for hiding this comment

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

Instead of manually calling zeImageDestory (which is error prone) it would be better to use v2::raii:ze_image_handle_t

@mateuszpn mateuszpn temporarily deployed to WindowsCILock May 6, 2025 12:34 — with GitHub Actions Inactive
@mateuszpn mateuszpn temporarily deployed to WindowsCILock May 7, 2025 12:12 — with GitHub Actions Inactive
@mateuszpn mateuszpn marked this pull request as ready for review May 8, 2025 08:00
@mateuszpn mateuszpn requested a review from a team as a code owner May 8, 2025 08:00
@mateuszpn
Copy link
Contributor Author

@intel/llvm-gatekeepers please merge, the issue is unrelated and tracked here: #18349

@kbenzie kbenzie merged commit dae689e into intel:sycl May 8, 2025
30 of 32 checks passed
@mateuszpn mateuszpn deleted the img-leak branch June 6, 2025 08:39
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.

4 participants