Skip to content

1. Use obsharedptr instead of shared_ptr. obsharedptr is #defined to the...#21

Merged
baoilleach merged 1 commit intoopenbabel:masterfrom
baoilleach:master
Aug 26, 2013
Merged

1. Use obsharedptr instead of shared_ptr. obsharedptr is #defined to the...#21
baoilleach merged 1 commit intoopenbabel:masterfrom
baoilleach:master

Conversation

@baoilleach
Copy link
Copy Markdown
Member

... appropriate value

  1. Add support for shared_ptrs to Python bindings. The following code now works:

r = OBReaction()
m = OBMol()
r.AddReactant(m)

…the appropriate value

2. Add support for shared_ptrs to Python bindings. The following code now works:

r = OBReaction()
m = OBMol()
r.AddReactant(m)
baoilleach added a commit that referenced this pull request Aug 26, 2013
1. Use obsharedptr instead of shared_ptr. obsharedptr is #defined to the...
@baoilleach baoilleach merged commit fbeecae into openbabel:master Aug 26, 2013
@Reinis
Copy link
Copy Markdown
Contributor

Reinis commented Nov 21, 2013

The commit message here could expand more on why the given Python snippet didn't work until now and why it does now.

I'm getting segfaults in shared_ptr destructor since this commit (or the one which fixes the build failure introduced in this one):

~$ python -c "import openbabel as ob; t = ob.OBMol(); s = ob.OBMol(); t += s"
Segmentation fault

Or in the interpreter:

~$ python
Python 3.3.2 (default, Nov 21 2013, 05:56:52) 
[GCC 4.8.2] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import openbabel as ob
>>> t = ob.OBMol()
>>> t += ob.OBMol()
>>> t += ob.OBMol()
Segmentation fault

This is backtrace:

#0  0x00000000006b5350 in ?? ()
No symbol table info available.
#1  0x00007ffff62ab9aa in std::tr1::_Sp_deleter<OpenBabel::OBMol>::operator() (this=0x6b5998, __p=0x81ec40)
    at /usr/lib/gcc/x86_64-pc-linux-gnu/4.8.2/include/g++-v4/tr1/shared_ptr.h:285
No locals.
#2  0x00007ffff62a5f47 in std::tr1::_Sp_counted_base_impl<OpenBabel::OBMol*, std::tr1::_Sp_deleter<OpenBabel::OBMol>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x6b5980) at /usr/lib/gcc/x86_64-pc-linux-gnu/4.8.2/include/g++-v4/tr1/shared_ptr.h:257
No locals.
#3  0x00007ffff624c02c in std::tr1::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x6b5980)
    at /usr/lib/gcc/x86_64-pc-linux-gnu/4.8.2/include/g++-v4/tr1/shared_ptr.h:141
No locals.
#4  0x00007ffff6229d69 in std::tr1::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x7724d8, __in_chrg=<optimized out>)
    at /usr/lib/gcc/x86_64-pc-linux-gnu/4.8.2/include/g++-v4/tr1/shared_ptr.h:341
No locals.
#5  0x00007ffff621cf44 in std::tr1::__shared_ptr<OpenBabel::OBMol, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x7724d0, 
    __in_chrg=<optimized out>) at /usr/lib/gcc/x86_64-pc-linux-gnu/4.8.2/include/g++-v4/tr1/shared_ptr.h:541
No locals.
#6  0x00007ffff621cf5e in std::tr1::shared_ptr<OpenBabel::OBMol>::~shared_ptr (this=0x7724d0, __in_chrg=<optimized out>)
    at /usr/lib/gcc/x86_64-pc-linux-gnu/4.8.2/include/g++-v4/tr1/shared_ptr.h:985
No locals.
#7  0x00007ffff611d8ff in _wrap_delete_OBMol (args=<SwigPyObject at remote 0x7ffff6b8ade0>)
    at /home/reinis/Hack/OpenBabel/openbabel-github/scripts/python/openbabel-python.cpp:29614
        argp1 = 0x7724d0
        swig_obj = {<SwigPyObject at remote 0x7ffff6b8ade0>}
        resultobj = 0x0
        arg1 = 0x81ec40
        res1 = 0
        tempshared1 = std::tr1::shared_ptr (empty) 0x0
        smartarg1 = 0x7724d0
#8  0x00007ffff606b194 in SwigPyObject_dealloc (v=<SwigPyObject at remote 0x7ffff6b8ade0>)
    at /home/reinis/Hack/OpenBabel/openbabel-github/scripts/python/openbabel-python.cpp:1704
        meth = 0x7ffff611d7ca <_wrap_delete_OBMol(PyObject*, PyObject*)>
        mself = <module at remote 0x7ffff6759f38>
        res = <type at remote 0x7ffff7d76220>
        ty = 0x7ffff66613a0 <_swigt__p_std__tr1__shared_ptrT_OpenBabel__OBMol_t>
        data = 0x779830
        destroy = <built-in method delete_OBMol of module object at remote 0x7ffff6759f38>
        sobj = 0x7ffff6b8ade0
        next = 0x0
#9  0x00007ffff7a0004f in free_keys_object (keys=keys@entry=0x675220)
    at /var/tmp/portage/dev-lang/python-3.3.2-r2/work/Python-3.3.2/Objects/dictobject.c:374
        entries = 0x675240
        i = 7
        n = 8
#10 0x00007ffff7a00b28 in dict_dealloc (mp=0x7ffff6c014d0) at /var/tmp/portage/dev-lang/python-3.3.2-r2/work/Python-3.3.2/Objects/dictobject.c:1392
        _tstate = 0x604680
        values = 0x0
        keys = 0x675220
        i = <optimized out>
        n = <optimized out>
#11 0x00007ffff7a19310 in subtype_dealloc (self=<OBMol(this=<SwigPyObject at remote 0x7ffff6b8ade0>) at remote 0x7ffff6c07750>)
    at /var/tmp/portage/dev-lang/python-3.3.2-r2/work/Python-3.3.2/Objects/typeobject.c:1024
        dict = <optimized out>
        dictptr = 0x7ffff6c07760
        _tstate = 0x604680
        type = <optimized out>
        base = 0x7ffff7d76220 <PyBaseObject_Type>
        basedealloc = 0x7ffff7a18a70 <object_dealloc>
        tstate = 0x604680
#12 0x00007ffff7a0184b in insertdict (mp=mp@entry=0x7ffff6c277a0, key='t', hash=-7895860575028071718, value=value@entry=None)
    at /var/tmp/portage/dev-lang/python-3.3.2-r2/work/Python-3.3.2/Objects/dictobject.c:824
        old_value = <optimized out>
        value_addr = 0x821cf0
        ep = 0x821ce0
#13 0x00007ffff7a0371a in PyDict_SetItem (
    op=op@entry={'__package__': None, '__loader__': None, '__doc__': None, '__name__': None, '__builtins__': <module at remote 0x7ffff7fe4440>, 't': N
one, 'ob': <module at remote 0x7ffff68d7908>}, key=<optimized out>, value=value@entry=None)
    at /var/tmp/portage/dev-lang/python-3.3.2-r2/work/Python-3.3.2/Objects/dictobject.c:1208
        mp = 0x7ffff6c277a0
        hash = <optimized out>
#14 0x00007ffff7a0b3fc in _PyModule_Clear (m=<optimized out>)
    at /var/tmp/portage/dev-lang/python-3.3.2-r2/work/Python-3.3.2/Objects/moduleobject.c:322
        pos = 11
        key = 't'
        value = <OBMol(this=<SwigPyObject at remote 0x7ffff6b8ade0>) at remote 0x7ffff6c07750>
        d = {'__package__': None, '__loader__': None, '__doc__': None, '__name__': None, '__builtins__': <module at remote 0x7ffff7fe4440>, 't': None,
 'ob': <module at remote 0x7ffff68d7908>}
#15 0x00007ffff7a88abd in PyImport_Cleanup () at /var/tmp/portage/dev-lang/python-3.3.2-r2/work/Python-3.3.2/Python/import.c:347
        pos = 140737351971893
        ndone = <optimized out>
        key = 0x0
        value = <module at remote 0x7ffff6beb128>
        dict = <optimized out>
        interp = 0x6045e0
        modules = {'encodings.utf_8': <module at remote 0x7ffff6c45e18>, '_thread': <module at remote 0x7ffff6c3ce18>, '_codecs': <module at remote 0x
7ffff6c58878>, 'DLFCN': <module at remote 0x7ffff6797680>, '__main__': <module at remote 0x7ffff6beb128>, 'mpl_toolkits': <module at remote 0x7ffff68b
2b48>, 'sre_compile': <module at remote 0x7ffff68f1050>, 'builtins': <module at remote 0x7ffff7fe4440>, 'zipimport': <module at remote 0x7ffff6c42680>
, 'sys': <module at remote 0x7ffff7ff2d88>, 'locale': <module at remote 0x7ffff68b23f8>, 'os.path': <module at remote 0x7ffff6c1aea8>, 'genericpath': 
<module at remote 0x7ffff6c227e8>, '_frozen_importlib': <module at remote 0x7ffff6c71290>, '_collections': <module at remote 0x7ffff6bd77a0>, 'codecs'
: <module at remote 0x7ffff6c4b200>, 'marshal': <module at remote 0x7ffff6c2d440>, 'posixpath': <module at remote 0x7ffff6c1aea8>, 'stat': <module at 
remote 0x7ffff6c1a4d0>, 'importlib._bootstrap': <module at remote 0x7ffff6c71290>, 'token': <module at remote 0x7ffff66ddf80>, 'importlib.mach...(trun
cated)
#16 0x00007ffff7a9305d in Py_Finalize () at /var/tmp/portage/dev-lang/python-3.3.2-r2/work/Python-3.3.2/Python/pythonrun.c:534
        interp = 0x6045e0
        tstate = 0x6b5350
#17 0x00007ffff7aa80e3 in Py_Main (argc=argc@entry=3, argv=argv@entry=0x603010)
    at /var/tmp/portage/dev-lang/python-3.3.2-r2/work/Python-3.3.2/Modules/main.c:773
        c = <optimized out>
        sts = 0
        command = 0x6043f0 L"\x653ed0"
        filename = 0x0
        module = 0x0
        fp = 0x7ffff77484e0 <_IO_2_1_stdin_>
        p = <optimized out>
        skipfirstline = <optimized out>
        stdin_is_interactive = 1
        help = <optimized out>
        version = <optimized out>
        saw_unbuffered_flag = <optimized out>
        cf = {cf_flags = 0}
        target_script_name = <optimized out>
#18 0x0000000000400e00 in main (argc=3, argv=<optimized out>) at /var/tmp/portage/dev-lang/python-3.3.2-r2/work/Python-3.3.2/Modules/python.c:81
        argv_copy = 0x603010
        argv_copy2 = 0x603040
        i = <optimized out>
        res = <optimized out>
        oldloc = 0x603070 "lv_LV.UTF-8"
        process_name = 0x0

SWIG generated wrapper code is really hard to follow, but it fails in openbabel-python.cpp:_wrap_delete_OBMol() calling delete smartarg1. The issue is that it doesn't fail there every time so it might be some bad interaction between shared_ptr and SWIG and Python. Any ideas?

@baoilleach
Copy link
Copy Markdown
Member Author

I'm prepared to revert my code if we can't sort this out, as I think mol += molb is more important than reactions, but let's see first.

"Why it didn't work till now?" The relevant function only takes a shared_ptr (OBReaction::AddReactant).
"Why does it work now?" All OBMol are now shared_ptrs. Yes, it's a bit crazy but I didn't notice any problems until now.

What to do? Maybe I should revert the change to openbabel-python.i for the moment and see if there's some other way to handle reactants. Chris is the best person to talk this over with, but he's not active at the moment...

(Note that the changes to the other files are still worthwhile and good. They bring all the shared_ptr handling to one point in our codebase.)

@Reinis
Copy link
Copy Markdown
Contributor

Reinis commented Nov 21, 2013

I think the best course of action now would be to back out the changes from Python bindings and commit them once the issue is understood and fixed. I checked that just reverting changes to openbabel-python.i is enough.

There are too many moving pieces in SWIG and shared_ptr, right now I don't completely understand how they interact.

@baoilleach
Copy link
Copy Markdown
Member Author

Please go ahead and commit it.

On 21 November 2013 21:41, Reinis notifications@github.com wrote:

I think the best course of action now would be to back out the changes
from Python bindings and commit them once the issue is understood and
fixed. I checked that just reverting changes to openbabel-python.i is
enough.

There are too many moving pieces in SWIG and shared_ptr, right now I don't
completely understand how they interact.


Reply to this email directly or view it on GitHubhttps://github.com//pull/21#issuecomment-29027309
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants