Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetSubstructMatch is not thread safe in Python #629

Closed
joekaus opened this issue Sep 30, 2015 · 4 comments
Closed

GetSubstructMatch is not thread safe in Python #629

joekaus opened this issue Sep 30, 2015 · 4 comments
Labels

Comments

@joekaus
Copy link

joekaus commented Sep 30, 2015

The Mol.GetSubstructMatch() method is not thread safe even when rdkit is build with RDK_BUILD_THREADSAFE_SSS=ON. Running the attached python code causes a segfault.

Python 2.7.10

BOOST 1.55.0

Using RDK_BUILD_THREADSAFE_SSS=ON, Rdkit Master (aba19e8)

from rdkit import Chem
import threading


def sub_match():
    ref_sdf = '\n     RDKit          3D\n\n 22 23  0  0  1  0  0  0  0  0999 V2000\n   -6.1917   -1.9517    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n   -5.0664   -1.3009    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n   -3.9401   -1.9499    0.0000 O   0  0  0  0  0  0  0  0  0  0  0  0\n   -2.8148   -1.2991    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n   -1.6885   -1.9483    0.0000 N   0  0  0  0  0  0  0  0  0  0  0  0\n   -0.5632   -1.2973    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n   -0.5642    0.0027    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n   -1.6905    0.6517    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n   -1.6916    1.9517    0.0000 N   0  0  0  0  0  0  0  0  0  0  0  0\n   -2.8158    0.0009    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n   -3.9422    0.6501    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n   -5.0685    1.2991    0.0000 N   0  0  0  0  0  0  0  0  0  0  0  0\n    0.5632   -1.9465    0.0000 N   0  0  0  0  0  0  0  0  0  0  0  0\n    1.6885   -1.2955    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n    1.6874    0.0046    0.0000 O   0  0  0  0  0  0  0  0  0  0  0  0\n    2.8148   -1.9447    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n    3.9401   -1.2936    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n    3.9391    0.0064    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n    5.0644    0.6572    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n    6.1907    0.0082    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n    6.1917   -1.2918    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n    5.0664   -1.9429    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\n  1  2  1  0\n  2  3  1  0\n  3  4  1  0\n  4 10  2  0\n  4  5  1  0\n  5  6  2  0\n  6  7  1  0\n  6 13  1  0\n  7  8  2  0\n  8  9  1  0\n  8 10  1  0\n 10 11  1  0\n 11 12  3  0\n 13 14  1  0\n 14 15  2  0\n 14 16  1  0\n 16 17  1  0\n 17 22  1  0\n 17 18  2  0\n 18 19  1  0\n 19 20  2  0\n 20 21  1  0\n 21 22  2  0\nM  END'
    core_smarts = '[#6]-!@[#6]-!@[#8]-!@[#6]:1:[#6](-!@[#6]#!@[#7]):[#6](-!@[#7]):[#6]:[#6](-!@[#7]-!@[#6](-!@[#6]-!@[#6]:2:[#6]:[#6]:[#6]:[#6]:[#6]:2)=!@[#8]):[#7]:1'
    ref_mol = Chem.MolFromMolBlock(ref_sdf)
    if ref_mol is None:
        raise ValueError('Bad ref structure')
    core_mol = Chem.MolFromSmarts(core_smarts)
    if core_mol is None:
        raise ValueError('Bad core structure')
    ref_mol.GetSubstructMatch(core_mol)

for i in xrange(1, 500):
    t = threading.Thread(target=sub_match, args=())
    t.start()

print "done"
@joekaus
Copy link
Author

joekaus commented Sep 30, 2015

Also, using rdkit-Release_2014_09_2 works fine

@greglandrum
Copy link
Member

I can reproduce this on the Mac working from master.

@bp-kelley
Copy link
Contributor

In the interim, we have released the GIL:

This probably exposed the thread-safety issues. In the short term we can roll this back (probably the reactions as well) for python.

@bp-kelley
Copy link
Contributor

This is fixed in this pull request. #637

I'm going through the other GIL releases and making sure the Python C/API is not called when the GIL is released. This is mostly done when returning convert the C++ results to python objects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants