From b0cc773ad9838608ed99c30932c18044550174fb Mon Sep 17 00:00:00 2001 From: Kevin Boyd Date: Wed, 10 Jun 2026 22:30:26 -0400 Subject: [PATCH] Fix GH issue 203: disconnected SMARTS crash with multithreaded preprocessing buildQueryBatchParallel ran addQueryToBatch inside an OpenMP parallel region. The validation error for fragment (disconnected) queries escaped the region and called std::terminate whenever more than one preprocessing thread was used, so the error only surfaced as a RuntimeError on the single-threaded path. Capture the first exception with OpenMPExceptionRegistry and rethrow once the region joins. --- nvmolkit/tests/test_substructure.py | 17 +++++++++++++++++ src/substruct/CMakeLists.txt | 2 +- src/substruct/molecules.cpp | 17 +++++++++++++++-- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/nvmolkit/tests/test_substructure.py b/nvmolkit/tests/test_substructure.py index 5d3fda33..2fd6161c 100644 --- a/nvmolkit/tests/test_substructure.py +++ b/nvmolkit/tests/test_substructure.py @@ -202,6 +202,23 @@ def test_buffer_overflow_rdkit_fallback(self): assert len(results[0][0]) == 10, f"Should get all 10 matches via RDKit fallback, got {len(results[0][0])}" assert matches_equal(results[0][0], rdkit_matches), "Matches should be identical to RDKit results" + @pytest.mark.parametrize("preprocessing_threads", [1, 2, -1]) + def test_disconnected_smarts_raises(self, preprocessing_threads: int): + """Disconnected (fragment) SMARTS must raise rather than crash the process. + + Regression test for GH issue 203: with more than one preprocessing + thread the validation error escaped the OpenMP region and terminated the + process. The unsupported-query error must surface as a RuntimeError for + every preprocessing-thread count. + """ + targets = [Chem.MolFromSmiles(smi) for smi in ("CC", "CCC", "CCCC")] + queries = [Chem.MolFromSmarts("C.C")] + + config = SubstructSearchConfig(preprocessingThreads=preprocessing_threads) + + with pytest.raises(RuntimeError, match="disconnected"): + hasSubstructMatch(targets, queries, config) + def test_no_match_possible(self): """Test when no match is possible.""" targets = [Chem.MolFromSmiles("CCCC")] diff --git a/src/substruct/CMakeLists.txt b/src/substruct/CMakeLists.txt index d52bb24d..b620bc08 100644 --- a/src/substruct/CMakeLists.txt +++ b/src/substruct/CMakeLists.txt @@ -22,7 +22,7 @@ target_include_directories(molecules PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) target_link_libraries( molecules PUBLIC device_vector flatBitVect OpenMP::OpenMP_CXX packed_bonds device - PRIVATE ${RDKit_LIBS} nvtx) + PRIVATE ${RDKit_LIBS} nvtx openmp_helpers) add_library(substruct_kernels substruct_kernels.cu) target_include_directories(substruct_kernels PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) diff --git a/src/substruct/molecules.cpp b/src/substruct/molecules.cpp index 3613b9b0..fc14e853 100644 --- a/src/substruct/molecules.cpp +++ b/src/substruct/molecules.cpp @@ -25,11 +25,13 @@ #include #include +#include #include #include #include #include "nvtx.h" +#include "openmp_helpers.h" #include "packed_bonds.h" #include "rdkit_compat.h" #include "substruct_debug.h" @@ -2128,6 +2130,12 @@ MoleculesHost buildQueryBatchParallel(const std::vector& mo threadBatches[t].reserve(molsPerThread, atomsPerThread); } + // addQueryToBatch validates each query and throws on unsupported inputs (e.g. + // disconnected SMARTS). An exception escaping the OpenMP region would call + // std::terminate, so capture the first failure and rethrow after the region + // joins. + detail::OpenMPExceptionRegistry exceptionRegistry; + #pragma omp parallel num_threads(numThreads) { const int tid = omp_get_thread_num(); @@ -2136,10 +2144,15 @@ MoleculesHost buildQueryBatchParallel(const std::vector& mo #pragma omp for schedule(static) for (int i = 0; i < numMols; ++i) { - const int molIdx = useSortOrder ? sortOrder[i] : i; - addQueryToBatch(molecules[molIdx], localBatch); + try { + const int molIdx = useSortOrder ? sortOrder[i] : i; + addQueryToBatch(molecules[molIdx], localBatch); + } catch (...) { + exceptionRegistry.store(std::current_exception()); + } } } + exceptionRegistry.rethrow(); MoleculesHost result; {