Fix: Cyles HIP-RT random crashes editing the scene

Perform delayed freeing of the geometry BVHs similar to OptiX.

Previously BLAS memory was allocated in the device class as part of
device_update, but released in the BVHHIPRT destructor which gets called
when deleting geometry outside of device_update.

To avoid the GPU accessing unmapped memory, do a delayed free of this
memory in the device class as part of either device_update or device
destruction. This ensures it is in sync with other device memory changes.

Fix #148276
Fix #139013
Fix #138043
Fix #140763

Pull Request: https://projects.blender.org/blender/blender/pulls/147247
This commit is contained in:
salipour
2025-10-17 16:32:23 +02:00
committed by Brecht Van Lommel
parent 58b8ae1717
commit b491002a9d
3 changed files with 35 additions and 5 deletions

View File

@@ -30,12 +30,10 @@ BVHHIPRT::BVHHIPRT(const BVHParams &params,
BVHHIPRT::~BVHHIPRT() BVHHIPRT::~BVHHIPRT()
{ {
HIPRTDevice *hiprt_device = static_cast<HIPRTDevice *>(device);
hiprtContext hiprt_context = hiprt_device->get_hiprt_context();
custom_primitive_bound.free(); custom_primitive_bound.free();
triangle_index.free(); triangle_index.free();
vertex_data.free(); vertex_data.free();
hiprtDestroyGeometry(hiprt_context, hiprt_geom); device->release_bvh(this);
} }
CCL_NAMESPACE_END CCL_NAMESPACE_END

View File

@@ -114,6 +114,7 @@ HIPRTDevice::HIPRTDevice(const DeviceInfo &info,
HIPRTDevice::~HIPRTDevice() HIPRTDevice::~HIPRTDevice()
{ {
HIPContextScope scope(this); HIPContextScope scope(this);
free_bvh_memory_delayed();
user_instance_id.free(); user_instance_id.free();
prim_visibility.free(); prim_visibility.free();
hiprt_blas_ptr.free(); hiprt_blas_ptr.free();
@@ -1150,12 +1151,33 @@ hiprtScene HIPRTDevice::build_tlas(BVHHIPRT *bvh,
return scene; return scene;
} }
void HIPRTDevice::free_bvh_memory_delayed()
{
thread_scoped_lock lock(hiprt_mutex);
if (stale_bvh.size()) {
for (int bvh_index = 0; bvh_index < stale_bvh.size(); bvh_index++) {
hiprtGeometry hiprt_geom = stale_bvh[bvh_index];
hiprtDestroyGeometry(hiprt_context, hiprt_geom);
hiprt_geom = nullptr;
}
stale_bvh.clear();
}
}
void HIPRTDevice::release_bvh(BVH *bvh)
{
BVHHIPRT *current_bvh = static_cast<BVHHIPRT *>(bvh);
thread_scoped_lock lock(hiprt_mutex);
/* Tracks BLAS pointers whose BVH destructors have been called. */
stale_bvh.push_back(current_bvh->hiprt_geom);
}
void HIPRTDevice::build_bvh(BVH *bvh, Progress &progress, bool refit) void HIPRTDevice::build_bvh(BVH *bvh, Progress &progress, bool refit)
{ {
if (have_error()) { if (have_error()) {
return; return;
} }
free_bvh_memory_delayed();
progress.set_substatus("Building HIPRT acceleration structure"); progress.set_substatus("Building HIPRT acceleration structure");
hiprtBuildOptions options; hiprtBuildOptions options;
@@ -1173,6 +1195,7 @@ void HIPRTDevice::build_bvh(BVH *bvh, Progress &progress, bool refit)
if (scene) { if (scene) {
hiprtDestroyScene(hiprt_context, scene); hiprtDestroyScene(hiprt_context, scene);
scene = nullptr;
} }
scene = build_tlas(bvh_rt, bvh_rt->objects, options, refit); scene = build_tlas(bvh_rt, bvh_rt->objects, options, refit);
} }

View File

@@ -46,6 +46,8 @@ class HIPRTDevice : public HIPDevice {
void build_bvh(BVH *bvh, Progress &progress, bool refit) override; void build_bvh(BVH *bvh, Progress &progress, bool refit) override;
void release_bvh(BVH *bvh) override;
hiprtContext get_hiprt_context() hiprtContext get_hiprt_context()
{ {
return hiprt_context; return hiprt_context;
@@ -65,7 +67,7 @@ class HIPRTDevice : public HIPDevice {
const vector<Object *> &objects, const vector<Object *> &objects,
hiprtBuildOptions options, hiprtBuildOptions options,
bool refit); bool refit);
void free_bvh_memory_delayed();
hiprtContext hiprt_context; hiprtContext hiprt_context;
hiprtScene scene; hiprtScene scene;
hiprtFuncTable functions_table; hiprtFuncTable functions_table;
@@ -74,6 +76,13 @@ class HIPRTDevice : public HIPDevice {
size_t scratch_buffer_size; size_t scratch_buffer_size;
device_vector<char> scratch_buffer; device_vector<char> scratch_buffer;
/* This vector tracks the hiprt_geom members of BVHRT so that device memory
* can be managed/released in HIPRTDevice.
* Even if synchronization occurs before memory release, a GPU job may still
* launch between synchronization and release, potentially causing the GPU
* to access unmapped memory. */
vector<hiprtGeometry> stale_bvh;
/* Is this scene using motion blur? Note there might exist motion data even if /* Is this scene using motion blur? Note there might exist motion data even if
* motion blur is disabled, for render passes. */ * motion blur is disabled, for render passes. */
bool use_motion_blur = false; bool use_motion_blur = false;