Source-Python-Dev-Team / Source.Python

This plugin aims to use boost::python and create an easily accessible wrapper around the Source Engine API for scripter use.
http://forums.sourcepython.com
GNU General Public License v3.0
164 stars 32 forks source link

TypeManager.pointer_attribute are broken due to incompatibility with the C/C++ memory ownership model. #490

Open CookStar opened 1 year ago

CookStar commented 1 year ago
  1. The values given to pointer_attribute are managed by CustomType's _allocated_pointers/_pointer_values, but since CustomType may be generated from raw pointers, CustomType cannot have ownership of the value. This applies to CustomType generated by make_object(), nested CustomType, and Array generated by static_pointer_array/dynamic_pointer_array. (See also #480, #489).

  2. If the value set in the pointer_attribute is an object generated by Python/Boost.Python, the memory will eventually be double free if the memory is managed by the C++ side. This can be circumvented by the user, but this is incompatible with the actual Source.Python implementation. Furthermore, this means that when the user handles a pointer_attribute, it must be fully aware of how the memory is managed. While this may be feasible at the library level, it is impractical at the user level.

These two problems eventually lead to segmentation violation or double free or memory corruption, and these problems are very difficult or impossible to debug.

Proposal:

We should not assign the pointer of an object generated by Python/Boost.Python directly to other memory location, we should always make a copy of it and assign it, as in an earlier implementation (https://github.com/Source-Python-Dev-Team/Source.Python/commit/7690d9674053a158bc79246dfc64d26a507d5209). https://github.com/Source-Python-Dev-Team/Source.Python/blob/7690d9674053a158bc79246dfc64d26a507d5209/addons/source-python/packages/source-python/memory/__init__.py#L394-L399

This means that _allocated_pointers/_pointer_values are no longer required. If the user needs to delete a copied object, they can always use CustomType._destructor.