core dump deleting atoms

[Greg Couch]

On Linux and Windows, but not always on Macs, deleting a chain will cause a core dump. For example, open 1a0m and delete chain A. The backtrace shows that it is trying to get the structure of a deleted atom.

Invalid read of size 8
   at 0x4183B80: atomstruct::Atom::structure() const (Atom.h:206)
   by 0x4184734: atomstruct::Bond::graphics_changes() const (Bond.h:119)
   by 0x41845FF: atomstruct::Bond::~Bond() (Bond.h:54)
   by 0x4184645: atomstruct::Bond::~Bond() (Bond.h:54)
   by 0x26274C73: atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}::operator()(atomstruct::Bond*) const (Structure.cpp:514)
   by 0x2627CDC1: bool __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}>::operator()<__gnu_cxx::__normal_iterator<atomstruct::Bond**, std::vector<atomstruct::Bond*, std::allocator<atomstruct::Bond*> > > >(__gnu_cxx::__normal_iterator<atomstruct::Bond**, std::vector<atomstruct::Bond*, std::allocator<atomstruct::Bond*> > >) (predefined_ops.h:241)
   by 0x2627D6CA: __gnu_cxx::__normal_iterator<atomstruct::Bond**, std::vector<atomstruct::Bond*, std::allocator<atomstruct::Bond*> > > std::__find_if<__gnu_cxx::__normal_iterator<atomstruct::Bond**, std::vector<atomstruct::Bond*, std::allocator<atomstruct::Bond*> > >, __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}> >(__gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}>, __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}>, __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}>, std::random_access_iterator_tag) (stl_algo.h:120)
   by 0x2627CD75: __gnu_cxx::__normal_iterator<atomstruct::Bond**, std::vector<atomstruct::Bond*, std::allocator<atomstruct::Bond*> > > std::__find_if<__gnu_cxx::__normal_iterator<atomstruct::Bond**, std::vector<atomstruct::Bond*, std::allocator<atomstruct::Bond*> > >, __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}> >(__gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}>, __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}>, __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}>) (stl_algo.h:162)
   by 0x2627C55A: __gnu_cxx::__normal_iterator<atomstruct::Bond**, std::vector<atomstruct::Bond*, std::allocator<atomstruct::Bond*> > > std::__remove_if<__gnu_cxx::__normal_iterator<atomstruct::Bond**, std::vector<atomstruct::Bond*, std::allocator<atomstruct::Bond*> > >, __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}> >(__gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}>, __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}>, __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}>) (stl_algo.h:860)
   by 0x2627C02D: __gnu_cxx::__normal_iterator<atomstruct::Bond**, std::vector<atomstruct::Bond*, std::allocator<atomstruct::Bond*> > > std::remove_if<__gnu_cxx::__normal_iterator<atomstruct::Bond**, std::vector<atomstruct::Bond*, std::allocator<atomstruct::Bond*> > >, atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}>(atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}, atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}, atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Bond*)#3}) (stl_algo.h:937)
   by 0x26275685: atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool) (Structure.cpp:516)
   by 0x26275866: atomstruct::Structure::delete_atoms(std::vector<atomstruct::Atom*, std::allocator<atomstruct::Atom*> > const&) (Structure.cpp:528)
   by 0x416F5ED: atom_delete (molc.cpp:470)
   by 0x21FECE17: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
   by 0x21FEC879: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
   by 0x21DD9C7E: _call_function_pointer (callproc.c:809)
   by 0x21DD9C7E: _ctypes_callproc (callproc.c:1147)
   by 0x21DD0B79: PyCFuncPtr_call (_ctypes.c:3870)

 Address 0x264928b0 is 224 bytes inside a block of size 240 free'd
   at 0x4C2F2AF: operator delete(void*) (vg_replace_malloc.c:576)
   by 0x262EA50B: atomstruct::Atom::~Atom() (Atom.cpp:56)
   by 0x26274B2D: atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Atom*)#2}::operator()(atomstruct::Atom*) const (Structure.cpp:495)
   by 0x2627CD0D: bool __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Atom*)#2}>::operator()<__gnu_cxx::__normal_iterator<atomstruct::Atom**, std::vector<atomstruct::Atom*, std::allocator<atomstruct::Atom*> > > >(__gnu_cxx::__normal_iterator<atomstruct::Atom**, std::vector<atomstruct::Atom*, std::allocator<atomstruct::Atom*> > >) (predefined_ops.h:241)
   by 0x2627C46E: __gnu_cxx::__normal_iterator<atomstruct::Atom**, std::vector<atomstruct::Atom*, std::allocator<atomstruct::Atom*> > > std::__remove_if<__gnu_cxx::__normal_iterator<atomstruct::Atom**, std::vector<atomstruct::Atom*, std::allocator<atomstruct::Atom*> > >, __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Atom*)#2}> >(__gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Atom*)#2}>, __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Atom*)#2}>, __gnu_cxx::__ops::_Iter_pred<atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Atom*)#2}>) (stl_algo.h:866)
   by 0x2627BFF5: __gnu_cxx::__normal_iterator<atomstruct::Atom**, std::vector<atomstruct::Atom*, std::allocator<atomstruct::Atom*> > > std::remove_if<__gnu_cxx::__normal_iterator<atomstruct::Atom**, std::vector<atomstruct::Atom*, std::allocator<atomstruct::Atom*> > >, atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Atom*)#2}>(atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Atom*)#2}, atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Atom*)#2}, atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool)::{lambda(atomstruct::Atom*)#2}) (stl_algo.h:937)
   by 0x2627536D: atomstruct::Structure::_delete_atoms(std::set<atomstruct::Atom*, std::less<atomstruct::Atom*>, std::allocator<atomstruct::Atom*> > const&, bool) (Structure.cpp:497)
   by 0x26275866: atomstruct::Structure::delete_atoms(std::vector<atomstruct::Atom*, std::allocator<atomstruct::Atom*> > const&) (Structure.cpp:528)
   by 0x416F5ED: atom_delete (molc.cpp:470)
   by 0x21FECE17: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
   by 0x21FEC879: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
   by 0x21DD9C7E: _call_function_pointer (callproc.c:809)
   by 0x21DD9C7E: _ctypes_callproc (callproc.c:1147)
   by 0x21DD0B79: PyCFuncPtr_call (_ctypes.c:3870)

 Block was alloc'd at
   at 0x4C2E325: operator new(unsigned long) (vg_replace_malloc.c:334)
   by 0x26276118: atomstruct::Structure::new_atom(char const*, element::Element const&) (Structure.cpp:622)
   by 0x2890F641: mmcif::ExtractMolecule::parse_atom_site() (mmcif.cpp:1331)
   by 0x289073E8: mmcif::ExtractMolecule::ExtractMolecule(_object*, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, bool)::{lambda()#5}::operator()() const (mmcif.cpp:355)
   by 0x289199BA: std::_Function_handler<void (), mmcif::ExtractMolecule::ExtractMolecule(_object*, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, bool)::{lambda()#5}>::_M_invoke(std::_Any_data const&) (functional:1731)
   by 0x289798F7: std::function<void ()>::operator()() const (functional:2127)
   by 0x289731EC: readcif::CIFFile::internal_parse(bool) (readcif.cpp:544)
   by 0x28972532: readcif::CIFFile::parse(char const*) (readcif.cpp:436)
   by 0x2897202E: readcif::CIFFile::parse_file(char const*) (readcif.cpp:393)
   by 0x289144BF: mmcif::parse_mmCIF_file(char const*, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, _object*, bool) (mmcif.cpp:2291)
   by 0x28971008: mmcif::_mmcif_parse_mmCIF_file(_object*, _object*, _object*) (_mmcif.cpp:239)
   by 0x4F151F3: _PyCFunction_FastCallDict (methodobject.c:231)

[Eric Pettersen]

The problem is the new graphics_changes()->set_gc_adddel() call in ~Bond. That call uses the bond's atoms, which may already been deleted.

Bond creation/deletion is completely controlled through Structure, so the set_gc_adddel call should be in Structure's delete_bond and delete_atom(s) function, which will not only avoid using deleted objects but will produce many fewer calls to set_gc_adddel.

--Eric

[Tom Goddard]

Fixed, I think. Could not reproduce the crash (working on Mac), but followed Eric's suggestion.