Residue.atoms yields corrupted array after residue deleted

[Tristan Croll]

If I have a model open:

m = session.models.list()[0]
r = m.residues[0]
session.models.close([m])
r.atoms.names

# yields either an array of garbage or a segmentation fault

[pett]

Hi Tristan,

I'm not sure what you want to happen here. You're holding a Python reference to a residue whose C++ half has been destroyed. For obvious performance reasons, we don't check the r.deleted property before each and every attribute access, so what should 'r.atoms' do/return in your usage scenario? You could of course check r.deleted yourself before trying to access r.atoms, or revamp your data in response to the atomic 'changes' trigger...

--Eric

[tic20@…]

I believe it used to be that deleting the C++ side of a Residue (or any State subclass) would delete the _cpp_pointer property of the object. So attempting to use it would cause a Python traceback but wouldn’t completely kill the session. I’ve already fixed the issue in my code, but reported it due to the changed behaviour.

 
 
Tristan Croll
Research Fellow
Cambridge Institute for Medical Research
University of Cambridge CB2 0XY
 

 

[pett]

That behavior was when Atom/Residues access was through ctypes, and the pointer reference occurred in Python code. Those classes are now implemented in Cython and the pointer access occurs in compiled code. I could put pointer checks into the Cython code so that incorrect user maintenance of Python data produces a traceback instead of crashes/corruption, at an unknown cost in performance.

So: maybe. It would require evaluation.

[goddard@…]

This is not good.  Crashing on atom attribute access from Python for a deleted atom is likely to cause many errors that cannot be solved because there was no traceback and the deletion might have happened 10 minutes, and 20 command earlier.  Not sure what we can do about this, but it needs some consideration.

[pett]

The cost of the check seems to be negligible, so I have added it. Unfortunately I can't abstract the pointer check into a cdef routine since those can't raise Python errors if they return non-Python values, so the checks are littered throughout the code.