Make ImplicitSharingPtr constructor from data pointer explicit

The ImplicitSharingPtr has an implicit constructor for raw pointers.
This has unintended effects when comparing an ImplicitSharingPtr to a
raw pointer: The raw pointer is implicitly converted to the shared
pointer (without change in refcount) and when going out of scope will
decrement user count, eventually freeing the data.

Conversion from raw pointer to shared pointer should not happen
implicitly. The constructor is made explicit now. This requires a little
more boilerplate when constructing a sharing pointer. A special
constructor for the nullptr is added so comparison with nullptr can
still happen without writing out a constructor.

Pull Request: https://projects.blender.org/blender/blender/pulls/115476
This commit is contained in:
Lukas Tönne
2023-11-27 15:53:29 +01:00
parent cf9987d548
commit 227a4eae77
6 changed files with 28 additions and 22 deletions

View File

@@ -42,19 +42,19 @@ GeometryComponentPtr GeometryComponent::create(Type component_type)
{
switch (component_type) {
case Type::Mesh:
return new MeshComponent();
return GeometryComponentPtr(new MeshComponent());
case Type::PointCloud:
return new PointCloudComponent();
return GeometryComponentPtr(new PointCloudComponent());
case Type::Instance:
return new InstancesComponent();
return GeometryComponentPtr(new InstancesComponent());
case Type::Volume:
return new VolumeComponent();
return GeometryComponentPtr(new VolumeComponent());
case Type::Curve:
return new CurveComponent();
return GeometryComponentPtr(new CurveComponent());
case Type::Edit:
return new GeometryComponentEditData();
return GeometryComponentPtr(new GeometryComponentEditData());
case Type::GreasePencil:
return new GreasePencilComponent();
return GeometryComponentPtr(new GreasePencilComponent());
}
BLI_assert_unreachable();
return {};
@@ -129,7 +129,7 @@ GeometryComponent &GeometrySet::get_component_for_write(GeometryComponent::Type
}
/* If the referenced component is shared, make a copy. The copy is not shared and is
* therefore mutable. */
component_ptr = component_ptr->copy();
component_ptr = GeometryComponentPtr(component_ptr->copy());
return *component_ptr;
}
@@ -185,7 +185,8 @@ void GeometrySet::add(const GeometryComponent &component)
{
BLI_assert(!components_[size_t(component.type())]);
component.add_user();
components_[size_t(component.type())] = const_cast<GeometryComponent *>(&component);
components_[size_t(component.type())] = GeometryComponentPtr(
const_cast<GeometryComponent *>(&component));
}
Vector<const GeometryComponent *> GeometrySet::get_components() const

View File

@@ -25,7 +25,10 @@ template<typename T> class ImplicitSharingPtr {
public:
ImplicitSharingPtr() = default;
ImplicitSharingPtr(T *data) : data_(data) {}
explicit ImplicitSharingPtr(T *data) : data_(data) {}
/* Implicit conversion from nullptr. */
ImplicitSharingPtr(std::nullptr_t) : data_(nullptr) {}
ImplicitSharingPtr(const ImplicitSharingPtr &other) : data_(other.data_)
{

View File

@@ -14,7 +14,7 @@ class ImplicitlySharedData : public ImplicitSharingMixin {
public:
ImplicitSharingPtr<ImplicitlySharedData> copy() const
{
return MEM_new<ImplicitlySharedData>(__func__);
return ImplicitSharingPtr<ImplicitlySharedData>(MEM_new<ImplicitlySharedData>(__func__));
}
void delete_self() override

View File

@@ -615,7 +615,8 @@ static void gather_realize_tasks_recursive(GatherTasksInfo &gather_info,
const auto *volume_component = static_cast<const bke::VolumeComponent *>(component);
if (!gather_info.r_tasks.first_volume) {
volume_component->add_user();
gather_info.r_tasks.first_volume = volume_component;
gather_info.r_tasks.first_volume = ImplicitSharingPtr<const bke::VolumeComponent>(
volume_component);
}
break;
}
@@ -624,7 +625,8 @@ static void gather_realize_tasks_recursive(GatherTasksInfo &gather_info,
component);
if (!gather_info.r_tasks.first_edit_data) {
edit_component->add_user();
gather_info.r_tasks.first_edit_data = edit_component;
gather_info.r_tasks.first_edit_data =
ImplicitSharingPtr<const bke::GeometryComponentEditData>(edit_component);
}
break;
}

View File

@@ -94,8 +94,8 @@ static std::shared_ptr<AnonymousAttributeFieldInput> make_attribute_field(
const NodeSimulationItem &item,
const CPPType &type)
{
AnonymousAttributeIDPtr attribute_id = MEM_new<NodeAnonymousAttributeID>(
__func__, self_object, compute_context, node, std::to_string(item.identifier), item.name);
AnonymousAttributeIDPtr attribute_id = AnonymousAttributeIDPtr(MEM_new<NodeAnonymousAttributeID>(
__func__, self_object, compute_context, node, std::to_string(item.identifier), item.name));
return std::make_shared<AnonymousAttributeFieldInput>(attribute_id, type, node.label_or_name());
}

View File

@@ -263,13 +263,13 @@ class LazyFunctionForGeometryNode : public LazyFunction {
}
}
const bNodeSocket &bsocket = node_.output_socket(output_bsocket_index);
AnonymousAttributeIDPtr attribute_id = MEM_new<NodeAnonymousAttributeID>(
__func__,
*this->get_self_object(*user_data),
*user_data->compute_context,
node_,
bsocket.identifier,
bsocket.name);
AnonymousAttributeIDPtr attribute_id = AnonymousAttributeIDPtr(
MEM_new<NodeAnonymousAttributeID>(__func__,
*this->get_self_object(*user_data),
*user_data->compute_context,
node_,
bsocket.identifier,
bsocket.name));
storage->attributes.append({output_bsocket_index, attribute_id});
return attribute_id;
};