Who manages STRING_ARRAY/STRING_POINTER?

[CookStar]

Who manages the char* set by Pointer.set_string_pointer? Is it impossible to set a string to a char* managed by a C++ object?

Test Code 1:

from memory import alloc

string = "string1"
string2 = "string2"

s_p1 = alloc(4, False)
s_p2 = alloc(4, False)

s_p1.set_string_pointer(string)
s_p2.set_string_pointer(string)

print("uint1", s_p1.get_uint())
print("uint2", s_p2.get_uint())

s_p1.set_string_pointer(s_p2.get_string_pointer())

print("uint1", s_p1.get_uint())
print("uint2", s_p2.get_uint())

print("string1", s_p1.get_string_pointer())
print("string2", s_p2.get_string_pointer())

s_p1.get_pointer().dealloc()

Output 1:

uint1 3949688248
uint2 3949688312
uint1 3953894104
uint2 3949688312
string1 12
string2 string2
free(): invalid pointer

First, I don't know why string1 is set to 12 instead of string2 as in string2, but trying to free string1 would result in an illegal pointer deallocation. Who exactly is managing this char*?

There is also a problem with TypeManager.pointer_attribute not being able to set STRING_ARRAY.

Test Code 2:

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

manager = TypeManager()

class Test(CustomType, metaclass=manager):
    _size = 4
    string = manager.pointer_attribute(Type.STRING_ARRAY, 0)

test = Test()
test.string = "test"

Output 2:

[Source.Python]
[SP] Caught an Exception:
Traceback (most recent call last):
  File "../addons/source-python/packages/source-python/plugins/command.py", line 164, in load_plugin
    plugin = self.manager.load(plugin_name)
  File "../addons/source-python/packages/source-python/plugins/manager.py", line 207, in load
    plugin._load()
  File "../addons/source-python/packages/source-python/plugins/instance.py", line 74, in _load
    self.module = import_module(self.import_name)
  File "../addons/source-python/plugins/test/test.py", line 4214, in <module>
    test.string = "test"
  File "../addons/source-python/packages/source-python/memory/manager.py", line 515, in fset
    instance_ptr = alloc(TYPE_SIZES[type_name.upper()])

KeyError: 'STRING_ARRAY'

STRING_ARRAY also allows buffer overrun, and it seems that STRING-related issues are somewhat problematic.

[jordanbriere]

char * is really just a pointer to an array. Perhaps this can give you some hints:

from memory import alloc

string = "string1"
string2 = "string2"

s_p1 = alloc(len(string) + 1, False)
s_p2 = alloc(len(string2) + 1, False)

s_p1.set_string_array(string)
s_p2.set_string_array(string2)

# struct { char *s1;, char *s2; }
p = alloc(8, False)
p.set_pointer(s_p1)
p.set_pointer(s_p2, 4)

print("p.s1", p.get_string_pointer(), p.get_pointer().get_string_array())
print("p.s2", p.get_string_pointer(4), p.get_pointer(4).get_string_array())

p.get_pointer().dealloc()
p.get_pointer(4).dealloc()

# These are now freed...
print(s_p1.get_string_array())
print(s_p2.get_string_array())

[CookStar]

char * is really just a pointer to an array. Perhaps this can give you some hints:

I understand how get/set_string_array works, it is set_string_pointer I am asking about.

Who manages the char* set by Pointer.set_string_pointer? Is it impossible to set a string to a char* managed by a C++ object?

[CookStar]

I will change the question, who determines the lifetime of this string object? When does it end?

from memory import alloc

s_p1 = alloc(4, False)
s_p1.set_string_pointer("string")

[jordanbriere]

I will change the question, who determines the lifetime of this string object? When does it end?

from memory import alloc

s_p1 = alloc(4, False)
s_p1.set_string_pointer("string")

s_p1.set_string_pointer("string") is the exact equivalence of s_p1.get_pointer().set_string_array("string"). Which obviously is undefined, because your memory is uninitialized and your pointer holds an unknown address.

[CookStar]

s_p1.set_string_pointer("string") is the exact equivalence of s_p1.get_pointer().set_string_array("string"). Which obviously is undefined, because your memory is uninitialized and your pointer holds an unknown address.

The actual behavior is not like that, set_string_pointer clearly sets the pointer of the string object.

Code:

from memory import alloc

p1 = memory.alloc(4, False)
print(p1.get_uint())
print(p1.get_pointer().address)
p1.set_string_pointer("string")
print(p1.get_uint())
print(p1.get_pointer().address)
print(p1.get_string_pointer())

p2 = memory.alloc(4, False)
print(p2.get_uint())
print(p2.get_pointer().address)

p2.get_pointer().set_string_array("string")

Output:

0
0
4023810712
4023810712
string
0
0
[Source.Python]
[SP] Caught an Exception:
Traceback (most recent call last):
  File "../addons/source-python/packages/source-python/plugins/command.py", line 164, in load_plugin
    plugin = self.manager.load(plugin_name)
  File "../addons/source-python/packages/source-python/plugins/manager.py", line 207, in load
    plugin._load()
  File "../addons/source-python/packages/source-python/plugins/instance.py", line 74, in _load
    self.module = import_module(self.import_name)
  File "../addons/source-python/plugins/test/test.py", line 4195, in <module>
    p2.get_pointer().set_string_array("string")

ValueError: Pointer is NULL.

[Ayuto]

I will change the question, who determines the lifetime of this string object? When does it end?

from memory import alloc

s_p1 = alloc(4, False)
s_p1.set_string_pointer("string")

s_p1.set_string_pointer("string") is the exact equivalence of s_p1.get_pointer().set_string_array("string"). Which obviously is undefined, because your memory is uninitialized and your pointer holds an unknown address.

This is not quite the equivalence. Let me bring some light into this topic by adding some comments to the very first example. set_string_pointer is actually a little bit dangerous, because you always have to keep a copy of the string alive in Python. As soon as your Python string is garbage collected, the pointer that has been set is also getting invalid. set_string_array is different, because it copies the bytes of the given string into an existing pointer.

from memory import alloc

string = "string1"
string2 = "string2"

s_p1 = alloc(4, False)
s_p2 = alloc(4, False)

# These two calls are perfectly fine, because your string is stored in a Python
# variable. When you delete the variable "string" or assign a new value,
# the pointer is invalidated and it's not clear what exactly is returned by
# get_string_pointer. But let's continue in the example...
s_p1.set_string_pointer(string)
s_p2.set_string_pointer(string)

print("uint1", s_p1.get_uint())
print("uint2", s_p2.get_uint())

# This will cause undefined behaviour, because get_string_pointer returns a new(!) Python string
# which is then set using set_string_pointer. After that line the returned string is 
# invalidated/forgotten by Python.
s_p1.set_string_pointer(s_p2.get_string_pointer())

# The following would work as we keep a copy of the string:
# new_string = s_p2.get_string_pointer()
# s_p1.set_string_pointer(new_string)

print("uint1", s_p1.get_uint())
print("uint2", s_p2.get_uint())

# Since s_p1 was set to a valid pointer, which was then invalidated, the result of
# s_p1.get_string_pointer() is now undefined.
print("string1", s_p1.get_string_pointer())
print("string2", s_p2.get_string_pointer())

# This will try to deallocated a pointer, which is not managed by you.
# This is the invalidated pointer that was managed by Python
s_p1.get_pointer().dealloc()

[jordanbriere]

I think this is an oversight of Pointer.set_string_pointer. The address is likely a capsule allocated by Boost, and its lifetime is valid only for the duration of the call.

[Ayuto]

STRING_ARRAY also allows buffer overrun [...]

Yes, this is something you need to take care about. We are not able to determine maximum possible length.

[CookStar]

This is not quite the equivalence. Let me bring some light into this topic by adding some comments to the very first example. set_string_pointer is actually a little bit dangerous, because you always have to keep a copy of the string alive in Python. As soon as your Python string is garbage collected, the pointer that has been set is also getting invalid. set_string_array is different, because it copies the bytes of the given string into an existing pointer.

Yes, this is something you need to take care about. We are not able to determine maximum possible length.

That seems to be the answer I was expecting, thank you.
So, it means we cannot safely use STRING_ARRAY/STRING_POINTER for instance_attribute/pointer_attribute in TypeManager.

[Ayuto]

Yes, reading is fine, but writing requires some extra caution at the moment.

[Ayuto]

@CookStar Do you have any further questions or can we close this issue?

[CookStar]

No more questions. Thanks for the answers. I have created a fix(#474) for some of the problems, so if you have any comments, please let me know.