Opened 7 years ago
Closed 7 years ago
#1096 closed defect (fixed)
Bug in c_array_property()
Reported by: | Tristan Croll | Owned by: | pett |
---|---|---|---|
Priority: | major | Milestone: | |
Component: | Core | Version: | |
Keywords: | Cc: | Tom Goddard | |
Blocked By: | Blocking: | ||
Notify when closed: | Platform: | all | |
Project: | ChimeraX |
Description
This is rather odd. Using the molc API, I have a class Proper_Dihedral
which contains the following:
atoms = c_property('proper_dihedral_atoms', cptr, 4, astype=_atoms, read_only=True, doc = 'Atoms making up this dihedral. Read only.')
This works exactly as expected to start with, but if I close one model and attempt to use it on the new one it fails silently by returning an empty Atoms
(Collection.__init__
just receives an empty array). I've added a reporter to my C++ side which shows it is returning the correct pointers in every case, and furthermore if I replace the above with the functionally-equivalent:
@property def atoms(self): f = c_function('proper_dihedral_atoms', args=(ctypes.c_void_p, ctypes.c_size_t, ctypes.c_void_p)) ret = numpy.empty(4, cptr) f(self._c_pointer_ref, 1, pointer(ret)) return _atoms(ret)
... then all is fine again, through opening and closing at least half a dozen models. Likewise, if I force c_property
to use c_varlen_property()
by doing:
@property def four(self): return 4 atoms = c_property('proper_dihedral_atoms', cptr, 'four', astype=_atoms, read_only=True, doc = 'Atoms making up this dihedral. Read only.')
... then everything works. Changing line 136 of molc.py
from
return astype(v)
to
return astype(v.copy())
seems to fix it. Not entirely sure why it arises in the first place, though.
Fixed.
Ugh. Collections do an evil thing, when C++ objects are deleted they resize the array of pointers held by the collection to a smaller size to eliminate the deleted items. They resize the array in-place, an operation that is not possible through normal numpy python calls. This breaks a basic assumption about how numpy arrays work. The molc.py property code had allocated a single numpy array for property return values and when that array length gets changed to 0 all future uses of that property have the wrong size. I added an a.copy() so that the property never exposes its internal array.
The removal of deleted C++ objects from C++ Collections should be improved to not resize numpy arrays in place. Replacing the Collection array pointer is also a bit perilous since there is no guarantee that Python code does not have other references to the original array and using that array with the deleted C++ objects will cause crashes. So it is not clear how to make the C++ object destruction handling of Collections more robust.