Subsequent call to rdChemReactions.ChemicalReaction.RunReactants will block indefinitely.

[goraj]

Describe the bug
Calling rdChemReactions.ChemicalReaction with the following SMIRKS will make any subsequent calls to RunReactants reusing the same rdChemReactions.ChemicalReaction object block indefinitely.
Work-around for now is to reinitialize a reaction before every use.

To Reproduce

import numpy as np
import rdkit
from rdkit.Chem import rdChemReactions
from rdkit.Chem.rdChemReactions import PreprocessReaction 
from rdkit.Chem import MolFromSmarts, MolFromSmiles
from rdkit.Chem import MolToSmarts, MolToSmiles

from typing import Tuple

def load_smirks(smirks: str) -> rdChemReactions.ChemicalReaction:
    func = rdChemReactions.ReactionFromSmarts(smirks)

    rdChemReactions.SanitizeRxn(func)
    func.Initialize()

    n_warnings, n_errors, n_reactants, n_products, labels = PreprocessReaction(func)
    if n_errors != 0:
        raise ValueError("Error in reaction initialization")
    return func

mol_sulfonylchloride = MolFromSmiles("Nc1c(CCCSNCC)cc(cc1)S(=O)(=O)Cl")
mol_amine = MolFromSmiles("Nc2cc(C)on2")
mol_sulfonamide = MolFromSmiles("CCNSCCCc1cc(S(=O)(=O)Nc2cc(C)on2)ccc1N")

smirks_fwd = (
    "[S;$(S(=O)(=O)[C,c,N]):1](=O)(=O)(-[Cl])"
     "."
     "[N;$([N&H2&D1,N&H1&D2])"
     # N aliphatic and not aromatic bond to carbon
     "&$(N(-&!@[#6]))"
     "&!$(N-C=[O,N,S]):2]"
     ">>"
     "[S:1](=O)(=O)-[N+0:2]"
)

smirks_reverse = ">>".join(smirks_fwd.split(">>")[::-1])

reaction_fwd = load_smirks(smirks_fwd)
reaction_reverse = load_smirks(smirks_reverse)


product = reaction_fwd.RunReactants((mol_sulfonylchloride, mol_amine))[0][0]
print("reaction_fwd", MolToSmiles(product))

reagent_sulfonylchloride, reagent_amine = reaction_reverse.RunReactants((mol_sulfonamide,))[0]

# trigger bug
print("trigger bug and mess up reaction_fwd object")
try:
    product = reaction_fwd.RunReactants((reagent_sulfonylchloride, reagent_amine))[0]
except RuntimeError as e:
    print(e)

print("call messed up reaction_fwd, this will inf loop")
# this will now lead to unstable behavior and never return
product = reaction_fwd.RunReactants((mol_sulfonylchloride, mol_amine))[0][0]
print(MolToSmiles(product))

Expected behavior
product = reaction_fwd.RunReactants((mol_sulfonylchloride, mol_amine))[0][0]
Should return.

Configuration (please complete the following information):

ProductName:	macOS
ProductVersion:	11.6
BuildVersion:	20G165
OSX, 20.6.0 Darwin Kernel Version 20.6.0: Mon Aug 30 06:12:21 PDT 2021; root:xnu-7195.141.6~3/RELEASE_X86_64 x86_64
Python 3.8.6
rdkit                     2021.03.5        py38h0bd8f9b_0    conda-forge

Cheers,
Jacob

[bp-kelley]

Ug, this has to be hitting one of the threading locks for the recursive smarts. I’ll take a look this weekend, thanks for the bug report!

…

---- Brian Kelley

On Oct 28, 2021, at 10:19 AM, Jacob ***@***.***> wrote:  Describe the bug Calling rdChemReactions.ChemicalReaction with the following SMIRKS will make any subsequent calls to RunReactants reusing the same rdChemReactions.ChemicalReaction object block indefinitely. Work-around for now is to reinitialize a reaction before every use. To Reproduce import numpy as np import rdkit from rdkit.Chem import rdChemReactions from rdkit.Chem.rdChemReactions import PreprocessReaction from rdkit.Chem import MolFromSmarts, MolFromSmiles from rdkit.Chem import MolToSmarts, MolToSmiles from typing import Tuple def load_smirks(smirks: str) -> rdChemReactions.ChemicalReaction: func = rdChemReactions.ReactionFromSmarts(smirks) rdChemReactions.SanitizeRxn(func) func.Initialize() n_warnings, n_errors, n_reactants, n_products, labels = PreprocessReaction(func) if n_errors != 0: raise ValueError("Error in reaction initialization") return func mol_sulfonylchloride = MolFromSmiles("Nc1c(CCCSNCC)cc(cc1)S(=O)(=O)Cl") mol_amine = MolFromSmiles("Nc2cc(C)on2") mol_sulfonamide = MolFromSmiles("CCNSCCCc1cc(S(=O)(=O)Nc2cc(C)on2)ccc1N") smirks_fwd = ( "[S;$(S(=O)(=O)[C,c,N]):1](=O)(=O)(-[Cl])" "." "[N;$([N&H2&D1,N&H1&D2])" # N aliphatic and not aromatic bond to carbon "&$(N(-&!@[#6]))" "&!$(N-C=[O,N,S]):2]" ">>" "[S:1](=O)(=O)-[N+0:2]" ) smirks_reverse = ">>".join(smirks_fwd.split(">>")[::-1]) reaction_fwd = load_smirks(smirks_fwd) reaction_reverse = load_smirks(smirks_reverse) product = reaction_fwd.RunReactants((mol_sulfonylchloride, mol_amine))[0][0] print("reaction_fwd", MolToSmiles(product)) reagent_sulfonylchloride, reagent_amine = reaction_reverse.RunReactants((mol_sulfonamide,))[0] # trigger bug print("trigger bug and mess up reaction_fwd object") try: product = reaction_fwd.RunReactants((reagent_sulfonylchloride, reagent_amine))[0] except RuntimeError as e: print(e) print("call messed up reaction_fwd, this will inf loop") # this will now lead to unstable behavior and never return product = reaction_fwd.RunReactants((mol_sulfonylchloride, mol_amine))[0][0] print(MolToSmiles(product)) Expected behavior product = reaction_fwd.RunReactants((mol_sulfonylchloride, mol_amine))[0][0] Should return. Configuration (please complete the following information): OSX, 20.6.0 Darwin Kernel Version 20.6.0: Mon Aug 30 06:12:21 PDT 2021; root:xnu-7195.141.6~3/RELEASE_X86_64 x86_64 Python 3.8.6 rdkit 2021.03.5 py38h0bd8f9b_0 conda-forge Cheers, Jacob — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[ptosco]

@bp-kelley Correct, I have noticed it deadlocks on a mutex with gdb.

[bp-kelley]

Looks like the fix is reasonable. When an exception is called internally during the substructure match in the reaction, we simply aren't unlocking the internal locks.

[bp-kelley]

@goraj a secondary issue is that the molecules coming out of reactions aren't sanitized. You can prevent this bug from occurring using:

Chem.SanitizeMol(reagent_sulfonylchloride)
Chem.SanitizeMol(reagent_amine)

Before calling Run_Reactants. The deadlock is still a bug though, but you'll need to do this anyway to actually run the reaction.

[greglandrum]

Yeah, we need to figure out some way to use RAII for those mutexes.
@bp-kelley : this is a mess I made, so let me know if you want me to do this one.

[bp-kelley]

@greglandrum the fast fix is easy, if you want to do an RAII that would be pretty simple as well.

[greglandrum]

I think RAII is the “right” way to do it; thanks for doing the PR!