Skip to content

Commit

Permalink
Refs #15831: Fix memory leaks in some tests
Browse files Browse the repository at this point in the history 
Signed-off-by: jparisu <[email protected]>
  • Loading branch information
jparisu committed Oct 13, 2022
1 parent 1a235a6 commit a3cf902
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/cpp/StatisticsBackendData.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ void StatisticsBackendData::stop_monitor(

// The monitor is inactive
// NOTE: for test sake, this is not always set
if (database_->is_entity_present(monitor_id))
if (database_ && database_->is_entity_present(monitor_id))
{
database_->change_entity_status(monitor_id, false);
}
Expand Down
7 changes: 7 additions & 0 deletions src/cpp/database/database.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,13 @@ class Database
{
public:

/**
* @brief Destructor of Database.
*
* @note It requires a virtual dtor for test sake.
*/
virtual ~Database() = default;

/**
* @brief Insert a new entity into the database.
* @param entity The entity object to be inserted.
Expand Down
2 changes: 2 additions & 0 deletions test/TrafficInjector/MessageSerializer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ class MessageSerializer
virtual void deserialize(
void* data,
const Message& message) = 0;

virtual ~MessageSerializer() = default;
};

class GuidMessageSerializer : public MessageSerializer
Expand Down
36 changes: 18 additions & 18 deletions test/TrafficInjector/TrafficInjector.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new WriterReaderDataMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<WriterReaderDataMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand All @@ -197,7 +197,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new Locator2LocatorDataMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<Locator2LocatorDataMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand All @@ -215,7 +215,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new EntityDataMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<EntityDataMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand All @@ -235,7 +235,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new EntityDataMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<EntityDataMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand All @@ -255,7 +255,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new Entity2LocatorTrafficMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<Entity2LocatorTrafficMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand All @@ -275,7 +275,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new Entity2LocatorTrafficMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<Entity2LocatorTrafficMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand All @@ -295,7 +295,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new EntityCountMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<EntityCountMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand Down Expand Up @@ -327,7 +327,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new EntityCountMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<EntityCountMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand Down Expand Up @@ -359,7 +359,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new EntityCountMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<EntityCountMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand Down Expand Up @@ -391,7 +391,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new EntityCountMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<EntityCountMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand Down Expand Up @@ -423,7 +423,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new EntityCountMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<EntityCountMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand Down Expand Up @@ -455,7 +455,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new EntityCountMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<EntityCountMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand Down Expand Up @@ -487,7 +487,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new EntityCountMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<EntityCountMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand Down Expand Up @@ -519,7 +519,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new EntityCountMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<EntityCountMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand Down Expand Up @@ -551,7 +551,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new DiscoveryTimeMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<DiscoveryTimeMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand All @@ -569,7 +569,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new SampleIdentityCountMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<SampleIdentityCountMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand All @@ -587,7 +587,7 @@ class TrafficInjector
type.register_type(participant_);
data_types_[kind] = type;

MessageSerializer* serializer = new PhysicalDataMessageSerializer();
std::shared_ptr<MessageSerializer> serializer = std::make_shared<PhysicalDataMessageSerializer>();
serializers_[kind] = serializer;

// Add the type to all other event kinds that use it
Expand Down Expand Up @@ -715,7 +715,7 @@ class TrafficInjector
std::map<StatisticsEventKind, Topic*> topics_;
std::map<StatisticsEventKind, TypeSupport> data_types_;

std::map<StatisticsEventKind, MessageSerializer*> serializers_;
std::map<StatisticsEventKind, std::shared_ptr<MessageSerializer>> serializers_;
std::map<StatisticsEventKind, std::string> event_names_;
std::map<std::string, StatisticsEventKind> event_kinds_;

Expand Down
15 changes: 11 additions & 4 deletions test/unittest/StatisticsBackend/IsActiveTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

using namespace eprosima::statistics_backend::subscriber;

bool StatisticsBackendTest::has_database_been_set_ = false;

/**
* @brief Fixture for the is_active_tests
* - Create a database loading it from a file.
Expand All @@ -36,7 +38,7 @@ class is_active_tests : public ::testing::Test

void SetUp()
{
db = new DataBaseTest;
db = new DataBaseTest; // This will be deleted inside StatisticsBackendTest unset_database
DatabaseDump dump;
load_file(EMPTY_ENTITIES_DUMP_FILE, dump);
db->load_database(dump);
Expand Down Expand Up @@ -106,6 +108,11 @@ class is_active_tests : public ::testing::Test
delete entity_queue;
delete data_queue;
delete participant_listener;

if (!StatisticsBackendTest::unset_database())
{
delete db;
}
}

std::shared_ptr<Host> host;
Expand All @@ -121,13 +128,13 @@ class is_active_tests : public ::testing::Test
DataBaseTest* db;

// Entity queue, attached to the database
DatabaseEntityQueue* entity_queue;
DatabaseEntityQueue* entity_queue = nullptr;
// Data queue, attached to the database
DatabaseDataQueue* data_queue;
DatabaseDataQueue* data_queue = nullptr;
// Statistics participant_, that is supposed to receive the callbacks
eprosima::fastdds::dds::DomainParticipant statistics_participant;
// Listener under tests. Will receive a pointer to statistics_participant
StatisticsParticipantListener* participant_listener;
StatisticsParticipantListener* participant_listener = nullptr;
};

// Windows dll do not export ParticipantProxyData class members (private APIs)
Expand Down
10 changes: 10 additions & 0 deletions test/unittest/StatisticsBackend/StatisticsBackendTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
using namespace eprosima::statistics_backend;
using namespace eprosima::statistics_backend::database;

bool StatisticsBackendTest::has_database_been_set_ = false;

/**
* @brief Fixture for the statistics_backend class tests
*
Expand All @@ -56,6 +58,14 @@ class statistics_backend_tests : public ::testing::TestWithParam<std::tuple<Enti
entities = PopulateDatabase::populate_database(*db);
}

void TearDown()
{
if (!StatisticsBackendTest::unset_database())
{
delete db;
}
}

DataBaseTest* db;
std::map<TestId, std::shared_ptr<const Entity>> entities;
};
Expand Down
31 changes: 31 additions & 0 deletions test/unittest/TestUtils/DatabaseUtils.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -634,8 +634,39 @@ class StatisticsBackendTest : public StatisticsBackend
monitor->domain_listener = nullptr;
details::StatisticsBackendData::get_instance()->monitors_by_entity_[domain->id] = monitor;
}
has_database_been_set_ = true;
}

/**
* @brief This method unset the internal database
*
* This method is required because the database is manually and hard set from the test, which makes
* ASAN cry.
* This will remove this instance in case it has been set, and do nothing otherwise.
*
* Be aware that in some cases where the database has not been set, it may be removed manually from
* the test. (This is why you should not create a smart ptr from a ptr initialized somewhere else.)
*
* @return true if the database has been reset
* @return false otherwise
*/
static bool unset_database()
{
if (has_database_been_set_)
{
details::StatisticsBackendData::get_instance()->database_.reset();
return true;
}
else
{
return false;
}
}

protected:

static bool has_database_been_set_;

};

/**
Expand Down

0 comments on commit a3cf902

Please sign in to comment.