Glassvision/IfcOpenShell
1
0
Fork 0
mirror of https://github.com/IfcOpenShell/IfcOpenShell.git synced 2026-08-10 17:58:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix memory leak for HeaderEntity instances:

Browse Source 
the getArgumentCount() call in IfcEntityInstanceData destructor isn't virtually dispatched so it returns 0 instead of correct number of arguments => memory leak
added IfcEntityInstanceData::clearArguments() which clears all arguments and resets the attributes_ field to nullptr

Remove unnecessary copy in IfcEntityInstanceData::setArgument() when argument is created at the call site (no more leak of original argument)

Fix memory leak in IfcFile: delete entities from both byid and entity_file_map
This commit is contained in:
Adrien SCHVALBERG
2022-04-28 10:33:54 +02:00
committed by Thomas Krijnen
parent e677355438
commit e5b3455533
4 changed files with 149 additions and 125 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ifcparse/IfcEntityInstanceData.h
+4 -2
View File
@@ -66,8 +66,8 @@ public:
Argument* getArgument(size_t i) const;
// NB: This makes a copy of the argument
void setArgument(size_t i, Argument* a, IfcUtil::ArgumentType attr_type = IfcUtil::Argument_UNKNOWN);
// NB: This makes a copy of the argument if make_copy is set
void setArgument(size_t i, Argument* a, IfcUtil::ArgumentType attr_type = IfcUtil::Argument_UNKNOWN, bool make_copy = false);
virtual size_t getArgumentCount() const {
if (type_ == 0) {
@@ -80,6 +80,8 @@ public:
}
}
void clearArguments();
const IfcParse::declaration* type() const {
return type_;
}
src/ifcparse/IfcParse.cpp
+140 -123
View File
@@ -1057,15 +1057,21 @@ std::string IfcEntityInstanceData::toString(bool upper) const {
return ss.str();
}
IfcEntityInstanceData::~IfcEntityInstanceData() {
void IfcEntityInstanceData::clearArguments()
{
if (attributes_ != NULL) {
for (size_t i = 0; i < getArgumentCount(); ++i) {
delete attributes_[i];
}
delete[] attributes_;
attributes_ = NULL;
}
}
IfcEntityInstanceData::~IfcEntityInstanceData() {
clearArguments();
}
unsigned IfcEntityInstanceData::set_id(boost::optional<unsigned> i) {
if (i) {
return id_ = *i;
@@ -1148,7 +1154,7 @@ IfcEntityInstanceData::IfcEntityInstanceData(const IfcEntityInstanceData& e) {
for (unsigned int i = 0; i < count; ++i) {
attributes_[i] = 0;
this->setArgument(i, e.getArgument(i), get_argument_type(e.type(), i));
this->setArgument(i, e.getArgument(i), get_argument_type(e.type(), i), true);
}
}
@@ -1265,131 +1271,135 @@ public:
};
void IfcEntityInstanceData::setArgument(size_t i, Argument* a, IfcUtil::ArgumentType attr_type) {
void IfcEntityInstanceData::setArgument(size_t i, Argument* a, IfcUtil::ArgumentType attr_type, bool make_copy) {
if (attributes_ == 0) {
load();
}
if (attr_type == IfcUtil::Argument_UNKNOWN) {
attr_type = a->type();
} else if (a->isNull()) {
attr_type = IfcUtil::Argument_NULL;
}
IfcWrite::IfcWriteArgument* copy = new IfcWrite::IfcWriteArgument();
switch (attr_type) {
case IfcUtil::Argument_NULL:
copy->set(boost::blank());
break;
case IfcUtil::Argument_DERIVED:
copy->set(IfcWrite::IfcWriteArgument::Derived());
break;
case IfcUtil::Argument_INT:
copy->set(static_cast<int>(*a));
break;
case IfcUtil::Argument_BOOL:
copy->set(static_cast<bool>(*a));
break;
case IfcUtil::Argument_LOGICAL: {
boost::logic::tribool tb = *a;
copy->set(tb);
break;
}
case IfcUtil::Argument_DOUBLE:
copy->set(static_cast<double>(*a));
break;
case IfcUtil::Argument_STRING:
copy->set(static_cast<std::string>(*a));
break;
case IfcUtil::Argument_BINARY: {
boost::dynamic_bitset<> attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_AGGREGATE_OF_INT: {
std::vector<int> attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_AGGREGATE_OF_DOUBLE: {
std::vector<double> attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_AGGREGATE_OF_STRING: {
std::vector<std::string> attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_AGGREGATE_OF_BINARY: {
std::vector< boost::dynamic_bitset<> > attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_ENUMERATION: {
std::string enum_literal = a->toString();
// Remove leading and trailing '.'
enum_literal = enum_literal.substr(1, enum_literal.size() - 2);
const IfcParse::enumeration_type* enum_type = type()->as_enumeration_type()
? type()->as_enumeration_type()
: type()->as_entity()->attribute_by_index(i)->type_of_attribute()->
as_named_type()->declared_type()->as_enumeration_type();
std::vector<std::string>::const_iterator it = std::find(
enum_type->enumeration_items().begin(),
enum_type->enumeration_items().end(),
enum_literal);
if (it == enum_type->enumeration_items().end()) {
throw IfcParse::IfcException(enum_literal + " does not name a valid item for " + enum_type->name());
Argument* new_attribute = a;
if (make_copy) {
if (attr_type == IfcUtil::Argument_UNKNOWN) {
attr_type = a->type();
} else if (a->isNull()) {
attr_type = IfcUtil::Argument_NULL;
}
copy->set(IfcWrite::IfcWriteArgument::EnumerationReference(it - enum_type->enumeration_items().begin(), it->c_str()));
break; }
case IfcUtil::Argument_ENTITY_INSTANCE: {
copy->set(static_cast<IfcUtil::IfcBaseClass*>(*a));
break; }
case IfcUtil::Argument_AGGREGATE_OF_ENTITY_INSTANCE: {
aggregate_of_instance::ptr instances = *a;
aggregate_of_instance::ptr mapped_instances(new aggregate_of_instance);
// @todo mapped_instances are not actually mapped to the file using add().
for (aggregate_of_instance::it it = instances->begin(); it != instances->end(); ++it) {
mapped_instances->push(*it);
IfcWrite::IfcWriteArgument* copy = new IfcWrite::IfcWriteArgument();
switch (attr_type) {
case IfcUtil::Argument_NULL:
copy->set(boost::blank());
break;
case IfcUtil::Argument_DERIVED:
copy->set(IfcWrite::IfcWriteArgument::Derived());
break;
case IfcUtil::Argument_INT:
copy->set(static_cast<int>(*a));
break;
case IfcUtil::Argument_BOOL:
copy->set(static_cast<bool>(*a));
break;
case IfcUtil::Argument_LOGICAL: {
boost::logic::tribool tb = *a;
copy->set(tb);
break;
}
copy->set(mapped_instances);
break; }
case IfcUtil::Argument_AGGREGATE_OF_AGGREGATE_OF_INT: {
std::vector< std::vector<int> > attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_AGGREGATE_OF_AGGREGATE_OF_DOUBLE: {
std::vector< std::vector<double> > attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_AGGREGATE_OF_AGGREGATE_OF_ENTITY_INSTANCE: {
aggregate_of_aggregate_of_instance::ptr instances = *a;
aggregate_of_aggregate_of_instance::ptr mapped_instances(new aggregate_of_aggregate_of_instance);
for (aggregate_of_aggregate_of_instance::outer_it it = instances->begin(); it != instances->end(); ++it) {
std::vector<IfcUtil::IfcBaseClass*> inner;
for (aggregate_of_aggregate_of_instance::inner_it jt = it->begin(); jt != it->end(); ++jt) {
inner.push_back(*jt);
case IfcUtil::Argument_DOUBLE:
copy->set(static_cast<double>(*a));
break;
case IfcUtil::Argument_STRING:
copy->set(static_cast<std::string>(*a));
break;
case IfcUtil::Argument_BINARY: {
boost::dynamic_bitset<> attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_AGGREGATE_OF_INT: {
std::vector<int> attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_AGGREGATE_OF_DOUBLE: {
std::vector<double> attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_AGGREGATE_OF_STRING: {
std::vector<std::string> attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_AGGREGATE_OF_BINARY: {
std::vector< boost::dynamic_bitset<> > attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_ENUMERATION: {
std::string enum_literal = a->toString();
// Remove leading and trailing '.'
enum_literal = enum_literal.substr(1, enum_literal.size() - 2);
const IfcParse::enumeration_type* enum_type = type()->as_enumeration_type()
? type()->as_enumeration_type()
: type()->as_entity()->attribute_by_index(i)->type_of_attribute()->
as_named_type()->declared_type()->as_enumeration_type();
std::vector<std::string>::const_iterator it = std::find(
enum_type->enumeration_items().begin(),
enum_type->enumeration_items().end(),
enum_literal);
if (it == enum_type->enumeration_items().end()) {
throw IfcParse::IfcException(enum_literal + " does not name a valid item for " + enum_type->name());
}
mapped_instances->push(inner);
}
copy->set(mapped_instances);
break; }
case IfcUtil::Argument_EMPTY_AGGREGATE:
case IfcUtil::Argument_AGGREGATE_OF_EMPTY_AGGREGATE: {
IfcUtil::ArgumentType t2 = IfcUtil::from_parameter_type(type()->as_entity()->attribute_by_index(i)->type_of_attribute());
delete copy;
copy = 0;
setArgument(i, a, t2);
break; }
default:
case IfcUtil::Argument_UNKNOWN:
throw IfcParse::IfcException(std::string("Unknown attribute encountered: '") + a->toString() + "' at index '" + boost::lexical_cast<std::string>(i) + "'");
break;
}
if (!copy) {
return;
copy->set(IfcWrite::IfcWriteArgument::EnumerationReference(it - enum_type->enumeration_items().begin(), it->c_str()));
break; }
case IfcUtil::Argument_ENTITY_INSTANCE: {
copy->set(static_cast<IfcUtil::IfcBaseClass*>(*a));
break; }
case IfcUtil::Argument_AGGREGATE_OF_ENTITY_INSTANCE: {
aggregate_of_instance::ptr instances = *a;
aggregate_of_instance::ptr mapped_instances(new aggregate_of_instance);
// @todo mapped_instances are not actually mapped to the file using add().
for (aggregate_of_instance::it it = instances->begin(); it != instances->end(); ++it) {
mapped_instances->push(*it);
}
copy->set(mapped_instances);
break; }
case IfcUtil::Argument_AGGREGATE_OF_AGGREGATE_OF_INT: {
std::vector< std::vector<int> > attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_AGGREGATE_OF_AGGREGATE_OF_DOUBLE: {
std::vector< std::vector<double> > attr_value = *a;
copy->set(attr_value);
break; }
case IfcUtil::Argument_AGGREGATE_OF_AGGREGATE_OF_ENTITY_INSTANCE: {
aggregate_of_aggregate_of_instance::ptr instances = *a;
aggregate_of_aggregate_of_instance::ptr mapped_instances(new aggregate_of_aggregate_of_instance);
for (aggregate_of_aggregate_of_instance::outer_it it = instances->begin(); it != instances->end(); ++it) {
std::vector<IfcUtil::IfcBaseClass*> inner;
for (aggregate_of_aggregate_of_instance::inner_it jt = it->begin(); jt != it->end(); ++jt) {
inner.push_back(*jt);
}
mapped_instances->push(inner);
}
copy->set(mapped_instances);
break; }
case IfcUtil::Argument_EMPTY_AGGREGATE:
case IfcUtil::Argument_AGGREGATE_OF_EMPTY_AGGREGATE: {
IfcUtil::ArgumentType t2 = IfcUtil::from_parameter_type(type()->as_entity()->attribute_by_index(i)->type_of_attribute());
delete copy;
copy = 0;
setArgument(i, a, t2, make_copy);
break; }
default:
case IfcUtil::Argument_UNKNOWN:
throw IfcParse::IfcException(std::string("Unknown attribute encountered: '") + a->toString() + "' at index '" + boost::lexical_cast<std::string>(i) + "'");
break;
}
if (!copy) {
return;
}
new_attribute = copy;
}
if (attributes_[i] != 0) {
@@ -1403,10 +1413,10 @@ void IfcEntityInstanceData::setArgument(size_t i, Argument* a, IfcUtil::Argument
if (this->file) {
register_inverse_visitor visitor(*this->file, *this);
apply_individual_instance_visitor(copy, i).apply(visitor);
apply_individual_instance_visitor(new_attribute, i).apply(visitor);
}
attributes_[i] = copy;
attributes_[i] = new_attribute;
}
//
@@ -2260,8 +2270,15 @@ IfcUtil::IfcBaseClass* IfcFile::instance_by_guid(const std::string& guid) {
// FIXME: Test destructor to delete entity and arg allocations
IfcFile::~IfcFile() {
for( entity_by_id_t::const_iterator it = byid.begin(); it != byid.end(); ++ it ) {
delete it->second;
std::set<IfcUtil::IfcBaseClass*> entities_to_delete;
for (const auto& pair : byid) {
entities_to_delete.insert(pair.second);
}
for (const auto& pair : entity_file_map) {
entities_to_delete.insert(pair.second);
}
for (auto entity : entities_to_delete) {
delete entity;
}
delete stream;
delete tokens;
src/ifcparse/IfcSpfHeader.cpp
+4
View File
@@ -46,6 +46,10 @@ HeaderEntity::HeaderEntity(const char * const datatype, size_t size, IfcFile* fi
}
}
HeaderEntity::~HeaderEntity()
{
clearArguments();
}
void IfcSpfHeader::readSemicolon() {
if (!TokenFunc::isOperator(file_->tokens->Next(), ';')) {
src/ifcparse/IfcSpfHeader.h
+1
View File
@@ -36,6 +36,7 @@ private:
HeaderEntity& operator =(const HeaderEntity&); //N/A
protected:
HeaderEntity(const char * const datatype, size_t size, IfcParse::IfcFile* file);
virtual ~HeaderEntity();
void setValue(unsigned int i, const std::string& s) {
IfcWrite::IfcWriteArgument* argument = new IfcWrite::IfcWriteArgument;