Fix importer leak: own Parts in ~ImportBase and delete the importer.
System::Load never deleted the ImportBase it allocated, and ~ImportBase was defaulted so the Parts container it held leaked too. Give ~ImportBase a body that deletes prts (the container only — the Part objects have been transferred to the Module via add(), which owns them; the default ~Parts frees the map without touching the elements, so no double free), and delete the importer at the end of System::Load (and on the early unreadable-file error path). Drop ImportMentor's explicit destructor, which called ImportBase::~ImportBase() in its body — harmless when the base dtor was empty, but a double delete of prts now that it frees memory. The implicit destructor calls the base once. Verified with valgrind on a batch load (mentor + a missing-file error path): "All heap blocks were freed — no leaks are possible", 0 errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -46,8 +46,11 @@ void System::Load(std::string module_name, std::string file_name, ImportType typ
|
|||||||
throw std::runtime_error("cannot open file: " + file_name);
|
throw std::runtime_error("cannot open file: " + file_name);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Creation or retrieval of the module, then parse into it.
|
// Creation or retrieval of the module, then parse into it. add() copies the
|
||||||
|
// Part pointers into the module, which takes ownership; deleting the
|
||||||
|
// importer then frees the (now drained) Parts container, not the parts.
|
||||||
Module *mod = mods->merge(module_name);
|
Module *mod = mods->merge(module_name);
|
||||||
imp->parse(mod->signals);
|
imp->parse(mod->signals);
|
||||||
mod->add(imp->parts());
|
mod->add(imp->parts());
|
||||||
|
delete imp;
|
||||||
}
|
}
|
||||||
@@ -64,9 +64,13 @@ public:
|
|||||||
/**
|
/**
|
||||||
* @brief Virtual destructor for ImportBase.
|
* @brief Virtual destructor for ImportBase.
|
||||||
*
|
*
|
||||||
* Ensures proper cleanup of derived classes.
|
* Frees the Parts container object. Only the container is deleted, not the
|
||||||
|
* Part objects it holds: by the time the importer is destroyed those have
|
||||||
|
* been transferred to a Module (SystemElementContainer::add copies the
|
||||||
|
* pointers), which owns and deletes them. The default ~Parts frees the map
|
||||||
|
* without touching the elements, so there is no double free.
|
||||||
*/
|
*/
|
||||||
virtual ~ImportBase() = default;
|
virtual ~ImportBase() { delete prts; }
|
||||||
};
|
};
|
||||||
|
|
||||||
#endif // _IMPORT_BASE_HPP_
|
#endif // _IMPORT_BASE_HPP_
|
||||||
@@ -43,16 +43,6 @@ enum class State
|
|||||||
*/
|
*/
|
||||||
ImportMentor::ImportMentor(string filename) : ImportBase(filename) {}
|
ImportMentor::ImportMentor(string filename) : ImportBase(filename) {}
|
||||||
|
|
||||||
/**
|
|
||||||
* @brief Destructor for ImportMentor.
|
|
||||||
*
|
|
||||||
* Ensures proper cleanup by calling the base class destructor.
|
|
||||||
*/
|
|
||||||
ImportMentor::~ImportMentor()
|
|
||||||
{
|
|
||||||
ImportBase::~ImportBase();
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @brief Parses the file to extract parts, pins, and signals.
|
* @brief Parses the file to extract parts, pins, and signals.
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -10,7 +10,6 @@ class ImportMentor : public ImportBase
|
|||||||
public:
|
public:
|
||||||
ImportMentor(std::string filename);
|
ImportMentor(std::string filename);
|
||||||
void parse(Signals *signals) override;
|
void parse(Signals *signals) override;
|
||||||
~ImportMentor();
|
|
||||||
};
|
};
|
||||||
|
|
||||||
#endif // _IMPORT_MENTOR_HPP_
|
#endif // _IMPORT_MENTOR_HPP_
|
||||||
Reference in New Issue
Block a user