ilusha/blender
1
0
Fork 0
mirror of https://github.com/blender/blender synced 2026-09-29 04:37:17 +03:00
Code Issues Projects Releases Packages Wiki Activity Actions

Fix #156334: Denoise node causes crashes

Browse source 
The Denoise node crashes Blender if denoising passes were connected
directly to a Denoise node inside and outside a node group at the same
time. That's due to double free where the derived data of the pass is
freed twice. This happens because denoising passes referenced external
data, and the result class assumed that external data would not have a
data reference count, and while that is true for the actual data, it is
not true for derived data, so data sharing might lead to derived data
getting freed twice.

To fix this, we keep track of data reference count even for external
data.

Pull Request: https://projects.blender.org/blender/blender/pulls/156368
This commit is contained in:
Omar Emara 2026-03-30 11:14:45 +02:00 • committed by Thomas Dinges
parent bac15f14bd
commit 77f4daa93e
2 changed files with 23 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

3
source/blender/compositor/COM_result.hh
View file

@ -132,8 +132,7 @@ class Result {
* member stores the number of results that share the data. This is heap allocated and have the
* same lifetime as allocated data, that's because this reference count is shared by all results
* that share the same data. Unlike the result's reference count, the data is freed if the count
* becomes 1, that is, data is no longer shared with some other result. This is nullptr if the
* data is external. */
* becomes 1, that is, data is no longer shared with some other result. */
int *data_reference_count_ = nullptr;
/* If the result is a single value, this member stores the value of the result, the value of
* which will be identical to that stored in the data_ member. The active variant member depends

40
source/blender/compositor/intern/result.cc
View file

@ -522,11 +522,7 @@ void Result::share_data(const Result &source)
*this = source;
reference_count_ = reference_count;
/* External data is intrinsically shared, and data_reference_count_ is nullptr in this case since
* it is not needed. */
if (!is_external_) {
(*data_reference_count_)++;
}
(*data_reference_count_)++;
}
void Result::steal_data(Result &source)
@ -581,6 +577,7 @@ void Result::wrap_external(gpu::Texture *texture)
is_external_ = true;
is_single_value_ = false;
domain_ = Domain(int2(GPU_texture_width(texture), GPU_texture_height(texture)));
data_reference_count_ = new int(1);
}
void Result::wrap_external(void *data, int2 size)
@ -592,6 +589,7 @@ void Result::wrap_external(void *data, int2 size)
storage_type_ = ResultStorageType::CPU;
is_external_ = true;
domain_ = Domain(size);
data_reference_count_ = new int(1);
}
void Result::wrap_external(const Result &result)
@ -605,6 +603,7 @@ void Result::wrap_external(const Result &result)
Result result_copy = result;
this->steal_data(result_copy);
is_external_ = true;
(*data_reference_count_)++;
}
void Result::set_transformation(const float3x3 &transformation)
@ -656,13 +655,6 @@ void Result::release()
void Result::free()
{
/* The data in the result are not owned by the result, so we only free the derived resources. */
if (is_external_) {
delete derived_resources_;
derived_resources_ = nullptr;
return;
}
if (!this->is_allocated()) {
return;
}
@ -688,6 +680,24 @@ void Result::free()
return;
}
delete data_reference_count_;
data_reference_count_ = nullptr;
delete derived_resources_;
derived_resources_ = nullptr;
if (is_external_) {
switch (storage_type_) {
case ResultStorageType::GPU:
gpu_texture_ = nullptr;
break;
case ResultStorageType::CPU:
cpu_data_ = GMutableSpan();
break;
}
return;
}
switch (storage_type_) {
case ResultStorageType::GPU:
if (is_from_pool_) {
@ -703,12 +713,6 @@ void Result::free()
cpu_data_ = GMutableSpan();
break;
}
delete data_reference_count_;
data_reference_count_ = nullptr;
delete derived_resources_;
derived_resources_ = nullptr;
}
bool Result::should_compute()