Skip to content

Commit 2b501a1

Browse files
author
Hugh Delaney
committed
Make urMemImageCreate failsafe
Use unique ptrs, and have the destructor call clear(), instead of calling it manually. Also add a case for a std::bad_alloc.
1 parent 3077fd4 commit 2b501a1

File tree

4 files changed

+35
-30
lines changed

4 files changed

+35
-30
lines changed

source/adapters/cuda/memory.cpp

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urMemRelease(ur_mem_handle_t hMem) {
102102
return UR_RESULT_SUCCESS;
103103
}
104104

105-
// make sure hMem is released in case checkErrorUR throws
106-
std::unique_ptr<ur_mem_handle_t_> MemObjPtr(hMem);
107-
108105
if (hMem->isSubBuffer()) {
109106
return UR_RESULT_SUCCESS;
110107
}
111108

112-
UR_CHECK_ERROR(hMem->clear());
109+
// Call destructor
110+
std::unique_ptr<ur_mem_handle_t_> MemObjPtr(hMem);
113111

114112
} catch (ur_result_t Err) {
115113
Result = Err;
@@ -230,13 +228,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urMemImageCreate(
230228
UR_ASSERT(pImageFormat->channelOrder == UR_IMAGE_CHANNEL_ORDER_RGBA,
231229
UR_RESULT_ERROR_UNSUPPORTED_IMAGE_FORMAT);
232230

233-
auto URMemObj = std::unique_ptr<ur_mem_handle_t_>(
234-
new ur_mem_handle_t_{hContext, flags, *pImageFormat, *pImageDesc, pHost});
235-
236-
UR_ASSERT(std::get<SurfaceMem>(URMemObj->Mem).PixelTypeSizeBytes,
237-
UR_RESULT_ERROR_UNSUPPORTED_IMAGE_FORMAT);
238-
239231
try {
232+
auto URMemObj = std::unique_ptr<ur_mem_handle_t_>(new ur_mem_handle_t_{
233+
hContext, flags, *pImageFormat, *pImageDesc, pHost});
234+
UR_ASSERT(std::get<SurfaceMem>(URMemObj->Mem).PixelTypeSizeBytes,
235+
UR_RESULT_ERROR_UNSUPPORTED_IMAGE_FORMAT);
236+
240237
if (PerformInitialCopy) {
241238
for (const auto &Device : hContext->getDevices()) {
242239
// Synchronous behaviour is best in this case
@@ -254,10 +251,10 @@ UR_APIEXPORT ur_result_t UR_APICALL urMemImageCreate(
254251

255252
*phMem = URMemObj.release();
256253
} catch (ur_result_t Err) {
257-
(*phMem)->clear();
258254
return Err;
255+
} catch (std::bad_alloc &Err) {
256+
return UR_RESULT_ERROR_OUT_OF_HOST_MEMORY;
259257
} catch (...) {
260-
(*phMem)->clear();
261258
return UR_RESULT_ERROR_UNKNOWN;
262259
}
263260
return UR_RESULT_SUCCESS;

source/adapters/cuda/memory.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,7 @@ struct ur_mem_handle_t_ {
394394
}
395395

396396
~ur_mem_handle_t_() {
397+
clear();
397398
if (isBuffer() && isSubBuffer()) {
398399
urMemRelease(std::get<BufferMem>(Mem).Parent);
399400
return;

source/adapters/hip/memory.cpp

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urMemRelease(ur_mem_handle_t hMem) {
6868
return UR_RESULT_SUCCESS;
6969
}
7070

71-
// make sure memObj is released in case UR_CHECK_ERROR throws
72-
std::unique_ptr<ur_mem_handle_t_> uniqueMemObj(hMem);
73-
7471
if (hMem->isSubBuffer()) {
7572
return UR_RESULT_SUCCESS;
7673
}
7774

78-
UR_CHECK_ERROR(hMem->clear());
75+
// Call destructor
76+
std::unique_ptr<ur_mem_handle_t_> uniqueMemObj(hMem);
7977

8078
} catch (ur_result_t Err) {
8179
Result = Err;
@@ -380,25 +378,33 @@ UR_APIEXPORT ur_result_t UR_APICALL urMemImageCreate(
380378
UR_ASSERT(pImageFormat->channelOrder == UR_IMAGE_CHANNEL_ORDER_RGBA,
381379
UR_RESULT_ERROR_UNSUPPORTED_IMAGE_FORMAT);
382380

383-
UR_CHECK_ERROR(checkSupportedImageChannelType(pImageFormat->channelType));
381+
try {
382+
UR_CHECK_ERROR(checkSupportedImageChannelType(pImageFormat->channelType));
384383

385-
auto URMemObj = std::unique_ptr<ur_mem_handle_t_>(
386-
new ur_mem_handle_t_{hContext, flags, *pImageFormat, *pImageDesc, pHost});
384+
auto URMemObj = std::unique_ptr<ur_mem_handle_t_>(new ur_mem_handle_t_{
385+
hContext, flags, *pImageFormat, *pImageDesc, pHost});
387386

388-
if (URMemObj == nullptr) {
389-
return UR_RESULT_ERROR_OUT_OF_HOST_MEMORY;
390-
}
387+
if (URMemObj == nullptr) {
388+
return UR_RESULT_ERROR_OUT_OF_HOST_MEMORY;
389+
}
391390

392-
if (PerformInitialCopy) {
393-
for (const auto &Dev : hContext->getDevices()) {
394-
ScopedContext Active(Dev);
395-
hipStream_t Stream{0}; // Use default stream
396-
UR_CHECK_ERROR(
397-
enqueueMigrateMemoryToDeviceIfNeeded(URMemObj.get(), Dev, Stream));
398-
UR_CHECK_ERROR(hipStreamSynchronize(Stream));
391+
if (PerformInitialCopy) {
392+
for (const auto &Dev : hContext->getDevices()) {
393+
ScopedContext Active(Dev);
394+
hipStream_t Stream{0}; // Use default stream
395+
UR_CHECK_ERROR(
396+
enqueueMigrateMemoryToDeviceIfNeeded(URMemObj.get(), Dev, Stream));
397+
UR_CHECK_ERROR(hipStreamSynchronize(Stream));
398+
}
399399
}
400+
*phMem = URMemObj.release();
401+
} catch (ur_result_t Err) {
402+
return Err;
403+
} catch (std::bad_alloc &Err) {
404+
return UR_RESULT_ERROR_OUT_OF_HOST_MEMORY;
405+
} catch (...) {
406+
return UR_RESULT_ERROR_UNKNOWN;
400407
}
401-
*phMem = URMemObj.release();
402408
return UR_RESULT_SUCCESS;
403409
}
404410

source/adapters/hip/memory.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,7 @@ struct ur_mem_handle_t_ {
390390
}
391391

392392
~ur_mem_handle_t_() noexcept(false) {
393+
clear();
393394
if (isBuffer() && isSubBuffer()) {
394395
urMemRelease(std::get<BufferMem>(Mem).Parent);
395396
return;

0 commit comments

Comments
 (0)