TypeManager.pointer_attribute cannot be safely used in nested CustomType.

[CookStar]

This is partially related to #472 and #474 though.

The values set in the TypeManager.pointer_attribute are managed by CustomType._allocated_pointers or CustomType._pointer_values, but if the original CustomType is managed by another object, they will be deallocated instantly. This means that pointer manipulation is not possible on nested CustomTypes.

My advice is we should stop using TypeManager.pointer_attribute. This feature has not been tested for a long time(#391) and should only be used in situations where you have full control over the memory of the values you are setting. (i.e. only non-native_type that you can manage yourself.) I found this out the hard way.

from memory.manager import CustomType
from memory.manager import TypeManager

manager = TypeManager()

class Test(CustomType, metaclass=manager):
    _size = 12

    test2 = manager.instance_attribute("Test2", 4)

class Test2(CustomType, metaclass=manager):
    _size = 8
    size = manager.pointer_attribute(Type.INT, 0)
    other_test = manager.pointer_attribute("Test", 4)

test = Test()
test.test2.size = 1 # BROKEN!, the memory of size is already deallocated.
test.test2.other_test = Test() # BROKEN!, the memory of Test is already deallocated.

[jordanbriere]

test.test2.size = 1 # BROKEN!, the memory of size is already deallocated.

That's not entirely accurate. The memory allocated for size is not freed yet because its setter still hold test2. After the assignment is done, however, test2 goes out of scope along with everything it allocated. For example:

test = Test()

test2 = test.test2
test2.size = 1

# Here test2.size is still held by test2._allocated_pointers...
del test2
# ...now size is being deallocated because test2 went out of scope.

So, the problem is not really pointer_attribute itself, but the fact that instance_attribute don't hold converted values:

Source.Python/addons/source-python/packages/source-python/memory/manager.py

Lines 446 to 450 in f129713

	def fget(ptr):
	"""Return the instance attribute value."""
	# Handle custom type
	if not native_type:
	return self.convert(type_name, ptr + offset)

If test2 was a pointer_attribute it would be held by test._pointer_values on assignment. Same goes for test2.other_test that is being held by test2._pointer_values which are also freed when test2 is no more. For instance, making Test.test2 cached would fix these issues:

class Test(CustomType, metaclass=manager):
    _size = 12

    test2 = cached_property(manager.instance_attribute("Test2", 4))

Because now test2 lifetime would be dependant of test itself.

[CookStar]

That's not entirely accurate. The memory allocated for size is not freed yet because its setter still hold test2. After the assignment is done, however, test2 goes out of scope along with everything it allocated.

I know, that is exactly what I was explaining in the comment I wrote.

class Test(CustomType, metaclass=manager):
    _size = 12

    test2 = cached_property(manager.instance_attribute("Test2", 4))

First, cached_property accepts getter and setter, not property, second, it is incompatible with data.ini, and third, that's just shuffling Python objects around to extend its lifetime. I understand objects lifetime, but others don't. That method cannot be used to prevent crashes.

The fundamental problem is that the _allocated_pointers and _pointer_values of CustomType are managing Python objects that should not be managed. Even if we use cached_property or some other method, there is no guarantee that the original Test2 can manage other objects in the first place (i.e. Test2 cannot contain Python objects since they can be created by make_object() at any time).

I wrote about the fundamental problem in #490, so I'll close this one.

[jordanbriere]

First, cached_property accepts getter and setter, not property,

cached_property(property) is the equivalence of CachedProperty(property.__get__. property.__set__, property.__delete__, property.__doc__) (via CachedProperty.wrap_descriptor).

[CookStar]

cached_property(property) is the equivalence of CachedProperty(property.__get__. property.__set__, property.__delete__, property.__doc__) (via CachedProperty.wrap_descriptor).

It seems I was testing with an old binary, my bad. But as noted above, cached_property is not a solution to this problem.