Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
CookStar opened this issue Aug 10, 2023 · 0 comments

Comments

@CookStar
Copy link
Contributor

CookStar commented Aug 10, 2023

  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 TypeManager.pointer_attribute cannot be safely used in nested CustomType. #480, TypeManager.static_pointer_array/TypeManager.dynamic_pointer_array are broken. #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 (7690d96).

# Q: Why do we use copy instead of set_ptr?
# A: Maybe it's a shared pointer which means that other
# classes might also have the address of it. So, changing
# address in this particular class wouldn't affect the
# other classes.
value.copy(ptr, self[attr_type].__size__)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant