Leak fixes for 2026.03.1 (#9198)

* fix mols leaked in tests

* own invariant generators

* clean up MorganFeatureAtomInvGenerator patterns

* address review suggestions
This commit is contained in:
Ricardo Rodriguez
2026-03-25 05:56:26 +01:00
committed by GitHub
parent cacba34a47
commit d90a73aa6b
11 changed files with 63 additions and 40 deletions

View File

@@ -594,7 +594,7 @@ TEST_CASE(
auto bond1 = m1->getBondWithIdx(1);
REQUIRE(bond1);
REQUIRE(bond1->hasQuery());
auto splitM1(
std::unique_ptr<ROMol> splitM1(
MolFragmenter::fragmentOnBonds(*m1, std::vector<unsigned int>{1, 5}));
REQUIRE(splitM1);
auto nb1 = splitM1->getBondWithIdx(11);
@@ -622,7 +622,7 @@ TEST_CASE(
auto bond2 = m2->getBondWithIdx(1);
REQUIRE(bond2);
REQUIRE(!bond2->hasQuery());
auto splitM2(
std::unique_ptr<ROMol> splitM2(
MolFragmenter::fragmentOnBonds(*m2, std::vector<unsigned int>{1}));
REQUIRE(splitM2);
auto nb5 = splitM2->getBondWithIdx(5);
@@ -641,8 +641,8 @@ TEST_CASE(
auto dummyLabels =
std::vector<std::pair<unsigned int, unsigned int>>{{25, 26}};
auto newTypes = std::vector<Bond::BondType>{Bond::DOUBLE};
auto splitM3(MolFragmenter::fragmentOnBonds(*m3, std::vector<unsigned int>{1},
true, &dummyLabels, &newTypes));
std::unique_ptr<ROMol> splitM3(MolFragmenter::fragmentOnBonds(
*m3, std::vector<unsigned int>{1}, true, &dummyLabels, &newTypes));
REQUIRE(splitM3);
auto nb7 = splitM3->getBondWithIdx(5);
REQUIRE(nb7);

View File

@@ -2701,15 +2701,17 @@ TEST_CASE("Read SD properties till last '>'") {
SDMolSupplier supplier;
supplier.setData(molblock);
auto m2 = supplier.next();
std::unique_ptr<ROMol> m2{supplier.next()};
REQUIRE(m2);
CHECK(m2->hasProp(prop_name));
}
TEST_CASE("github9101 - $$$$ at buffer end") {
std::string infile = rdbase + "/Code/GraphMol/FileParsers/test_data/rdkit_chunk_boundary_bug.sdf";
std::string infile =
rdbase +
"/Code/GraphMol/FileParsers/test_data/rdkit_chunk_boundary_bug.sdf";
SDMolSupplier reader(infile);
CHECK(reader.length() == 2); // this causes the issue as we pre-index
CHECK(reader.length() == 2); // this causes the issue as we pre-index
auto *mol = reader[0];
CHECK(mol->getProp<std::string>("comment").size() == 65369);
delete mol;

View File

@@ -317,7 +317,7 @@ std::unique_ptr<FingerprintGenerator<std::uint64_t>> generatorFromJSON(
return std::make_unique<FingerprintGenerator<std::uint64_t>>(
envGen.release(), fpArgs.release(),
atomInvGen ? atomInvGen.release() : nullptr,
bondInvGen ? bondInvGen.release() : nullptr);
bondInvGen ? bondInvGen.release() : nullptr, true, true);
}
template <typename OutputType>

View File

@@ -209,32 +209,30 @@ typedef boost::flyweight<boost::flyweights::key_value<std::string, ss_matcher>,
boost::flyweights::no_tracking>
pattern_flyweight;
void getFeatureInvariants(const ROMol &mol, std::vector<uint32_t> &invars,
std::vector<const ROMol *> *patterns) {
const std::vector<const ROMol *> *patterns) {
unsigned int nAtoms = mol.getNumAtoms();
PRECONDITION(invars.size() >= nAtoms, "vector too small");
auto useLocalPatterns = patterns == nullptr;
std::vector<const ROMol *> featureMatchers;
if (!patterns) {
if (useLocalPatterns) {
featureMatchers.reserve(defaultFeatureSmarts.size());
for (std::vector<std::string>::const_iterator smaIt =
defaultFeatureSmarts.begin();
smaIt != defaultFeatureSmarts.end(); ++smaIt) {
const ROMol *matcher = pattern_flyweight(*smaIt).get().getMatcher();
for (const auto &smaIt : defaultFeatureSmarts) {
const ROMol *matcher = pattern_flyweight(smaIt).get().getMatcher();
CHECK_INVARIANT(matcher, "bad smarts");
featureMatchers.push_back(matcher);
}
patterns = &featureMatchers;
}
std::fill(invars.begin(), invars.end(), 0);
for (unsigned int i = 0; i < patterns->size(); ++i) {
auto &queries = (useLocalPatterns ? featureMatchers : *patterns);
for (unsigned int i = 0; i < queries.size(); ++i) {
unsigned int mask = 1 << i;
std::vector<MatchVectType> matchVect;
// to maintain thread safety, we have to copy the pattern
// molecules:
SubstructMatch(mol, ROMol(*(*patterns)[i], true), matchVect);
for (std::vector<MatchVectType>::const_iterator mvIt = matchVect.begin();
mvIt != matchVect.end(); ++mvIt) {
for (const auto &mIt : *mvIt) {
SubstructMatch(mol, ROMol(*queries[i], true), matchVect);
for (const auto &mvIt : matchVect) {
for (const auto &mIt : mvIt) {
invars[mIt.second] |= mask;
}
}

View File

@@ -128,7 +128,7 @@ const std::string morganConnectivityInvariantVersion = "1.0.0";
*/
RDKIT_FINGERPRINTS_EXPORT void getFeatureInvariants(
const ROMol &mol, std::vector<std::uint32_t> &invars,
std::vector<const ROMol *> *patterns = nullptr);
const std::vector<const ROMol *> *patterns = nullptr);
const std::string morganFeatureInvariantVersion = "0.1.0";
} // namespace MorganFingerprints

View File

@@ -68,8 +68,28 @@ MorganAtomInvGenerator *MorganAtomInvGenerator::clone() const {
}
MorganFeatureAtomInvGenerator::MorganFeatureAtomInvGenerator(
std::vector<const ROMol *> *patterns) {
dp_patterns = patterns;
const std::vector<const ROMol *> *patterns) {
if (patterns) {
dp_patterns = new std::vector<const ROMol *>;
dp_patterns->reserve(patterns->size());
for (auto pattern : *patterns) {
dp_patterns->push_back(new ROMol(*pattern));
}
}
}
void MorganFeatureAtomInvGenerator::cleanUpPatterns() {
if (dp_patterns) {
for (auto mol : *dp_patterns) {
delete mol;
}
delete dp_patterns;
dp_patterns = nullptr;
}
}
MorganFeatureAtomInvGenerator::~MorganFeatureAtomInvGenerator() {
cleanUpPatterns();
}
std::string MorganFeatureAtomInvGenerator::infoString() const {
@@ -94,6 +114,7 @@ void MorganFeatureAtomInvGenerator::fromJSON(
const boost::property_tree::ptree &pt) {
if (pt.get_child_optional("patternSMARTS")) {
const auto &patternsNode = pt.get_child("patternSMARTS");
cleanUpPatterns();
dp_patterns = new std::vector<const ROMol *>();
for (const auto &patternNode : patternsNode) {
std::string smarts = patternNode.second.get_value<std::string>();
@@ -543,12 +564,12 @@ FingerprintGenerator<OutputType> *getMorganGenerator(
ownsBondInvGen);
}
template RDKIT_FINGERPRINTS_EXPORT FingerprintGenerator<std::uint32_t> *
getMorganGenerator(const MorganArguments &, AtomInvariantsGenerator *,
BondInvariantsGenerator *, bool, bool);
template RDKIT_FINGERPRINTS_EXPORT FingerprintGenerator<std::uint64_t> *
getMorganGenerator(const MorganArguments &, AtomInvariantsGenerator *,
BondInvariantsGenerator *, bool, bool);
template RDKIT_FINGERPRINTS_EXPORT FingerprintGenerator<std::uint32_t>
*getMorganGenerator(const MorganArguments &, AtomInvariantsGenerator *,
BondInvariantsGenerator *, bool, bool);
template RDKIT_FINGERPRINTS_EXPORT FingerprintGenerator<std::uint64_t>
*getMorganGenerator(const MorganArguments &, AtomInvariantsGenerator *,
BondInvariantsGenerator *, bool, bool);
template RDKIT_FINGERPRINTS_EXPORT FingerprintGenerator<std::uint32_t> *
getMorganGenerator(unsigned int radius, bool countSimulation,

View File

@@ -14,7 +14,6 @@
#include <GraphMol/Fingerprints/FingerprintGenerator.h>
#include <cstdint>
namespace RDKit {
namespace MorganFingerprint {
@@ -53,7 +52,8 @@ class RDKIT_FINGERPRINTS_EXPORT MorganAtomInvGenerator
*/
class RDKIT_FINGERPRINTS_EXPORT MorganFeatureAtomInvGenerator
: public AtomInvariantsGenerator {
std::vector<const ROMol *> *dp_patterns;
void cleanUpPatterns();
std::vector<const ROMol *> *dp_patterns = nullptr;
public:
/**
@@ -64,7 +64,9 @@ class RDKIT_FINGERPRINTS_EXPORT MorganFeatureAtomInvGenerator
Gobbi and Poppinger, Biotech. Bioeng. _61_ 47-54 (1998) will be used for
Donor, Acceptor, Aromatic, Halogen, Basic, Acidic.
*/
MorganFeatureAtomInvGenerator(std::vector<const ROMol *> *patterns = nullptr);
MorganFeatureAtomInvGenerator(
const std::vector<const ROMol *> *patterns = nullptr);
~MorganFeatureAtomInvGenerator();
std::vector<std::uint32_t> *getAtomInvariants(
const ROMol &mol) const override;

View File

@@ -58,10 +58,9 @@ AtomInvariantsGenerator *getMorganAtomInvGen(const bool includeRingMembership) {
AtomInvariantsGenerator *getMorganFeatureAtomInvGen(
python::object &py_patterns) {
std::vector<const ROMol *> patterns;
python::extract<std::vector<const ROMol *>> patternsE(py_patterns);
if (patternsE.check()) {
patterns = patternsE();
const auto &patterns = patternsE();
return new MorganFingerprint::MorganFeatureAtomInvGenerator(&patterns);
} else {
return new MorganFingerprint::MorganFeatureAtomInvGenerator(nullptr);

View File

@@ -818,7 +818,8 @@ TEST_CASE("toJSON") {
CHECK(!jsonStr.empty());
CHECK(jsonStr.find("\"type\":\"MorganArguments\"") != std::string::npos);
auto fpGenerator2 = generatorFromJSON(jsonStr);
std::unique_ptr<FingerprintGenerator<std::uint64_t>> fpGenerator2 =
generatorFromJSON(jsonStr);
REQUIRE(fpGenerator2);
std::unique_ptr<ExplicitBitVect> fp2{fpGenerator2->getFingerprint(*m1)};
REQUIRE(fp2);

View File

@@ -430,10 +430,10 @@ TEST_CASE("test9", "[substruct][chiral]") {
auto q1 = std::make_unique<RWMol>();
q1->addAtom(a6.get());
q1->addAtom(new Atom(6));
q1->addAtom(new Atom(7));
q1->addAtom(new Atom(8));
q1->addAtom(new Atom(9));
q1->addAtom(new Atom(6), updateLabel, takeOwnership);
q1->addAtom(new Atom(7), updateLabel, takeOwnership);
q1->addAtom(new Atom(8), updateLabel, takeOwnership);
q1->addAtom(new Atom(9), updateLabel, takeOwnership);
q1->addBond(0, 1, Bond::SINGLE);
q1->addBond(0, 2, Bond::SINGLE);
q1->addBond(0, 3, Bond::SINGLE);

View File

@@ -4990,7 +4990,7 @@ TEST_CASE("canonical re-kekulization after sanitization preserves stereo",
"C/C=C/c1ccc2ncccc2c1");
CAPTURE(smiles);
auto mol = SmilesToMol(smiles);
auto mol = v2::SmilesParse::MolFromSmiles(smiles);
REQUIRE(mol);
auto refSmi = MolToSmiles(*mol);