Enhance memory leak detection (#245)

* Change Debug -> COVERAGE flag and add fsanitize=address

* Add virtual destructor to prevent memory leaks

* Make m_agents private for security reasons

* Refactor one line

* Fix compile-time warnings

* Update version

* Fix apple-clang warning

Author: Grufoony

.github/workflows/cmake_test.yml

@@ -7,7 +7,7 @@ on:
     branches: [ "main" ]
 
 env:
-  BUILD_TYPE: Debug
+  BUILD_TYPE: COVERAGE
 
 jobs:
   build:

.github/workflows/cmake_test_macos.yml

@@ -6,9 +6,6 @@ on:
   pull_request:
     branches: [ "main" ]
 
-env:
-  BUILD_TYPE: Debug
-
 jobs:
   build:
     runs-on: macos-latest
@@ -18,12 +15,12 @@ jobs:
 
     - name: Configure CMake
       working-directory: ${{github.workspace}}/test
-      run: cmake -B ${{github.workspace}}/test/build -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}}
+      run: cmake -B ${{github.workspace}}/test/build
 
     - name: Build
       working-directory: ${{github.workspace}}/test
       # Build your program with the given configuration
-      run: cmake --build ${{github.workspace}}/test/build --config ${{env.BUILD_TYPE}}
+      run: cmake --build ${{github.workspace}}/test/build
 
     - name: Run tests
       working-directory: ${{github.workspace}}/test

src/dsm/dsm.hpp

@@ -6,7 +6,7 @@
 
 static constexpr uint8_t DSM_VERSION_MAJOR = 2;
 static constexpr uint8_t DSM_VERSION_MINOR = 3;
-static constexpr uint8_t DSM_VERSION_PATCH = 0;
+static constexpr uint8_t DSM_VERSION_PATCH = 1;
 
 static auto const DSM_VERSION =
     std::format("{}.{}.{}", DSM_VERSION_MAJOR, DSM_VERSION_MINOR, DSM_VERSION_PATCH);

src/dsm/headers/Dynamics.hpp

@@ -64,9 +64,11 @@ namespace dsm {
   /// @tparam Size, The type of the graph's capacity. It must be an unsigned integral type.
   template <typename agent_t>
   class Dynamics {
+  private:
+    std::map<Id, std::unique_ptr<agent_t>> m_agents;
+
   protected:
     std::unordered_map<Id, std::unique_ptr<Itinerary>> m_itineraries;
-    std::map<Id, std::unique_ptr<agent_t>> m_agents;
     Graph m_graph;
     Time m_time, m_previousSpireTime;
     std::mt19937_64 m_generator;

src/dsm/headers/FirstOrderDynamics.cpp

@@ -27,7 +27,7 @@ namespace dsm {
   }
 
   void FirstOrderDynamics::setAgentSpeed(Size agentId) {
-    const auto& agent{this->m_agents[agentId]};
+    const auto& agent{this->agents().at(agentId)};
     const auto& street{this->m_graph.streetSet()[agent->streetId().value()]};
     double speed{street->maxSpeed() * (1. - m_alpha * street->density(true))};
     if (m_speedFluctuationSTD > 0.) {
@@ -59,12 +59,12 @@ namespace dsm {
       meanSpeed = street->maxSpeed() * n * (1. - 0.5 * alpha * (n - 1.));
     } else {
       for (const auto& agentId : street->waitingAgents()) {
-        meanSpeed += this->m_agents.at(agentId)->speed();
+        meanSpeed += this->agents().at(agentId)->speed();
         ++n;
       }
       for (auto const& queue : street->exitQueues()) {
         for (const auto& agentId : queue) {
-          meanSpeed += this->m_agents.at(agentId)->speed();
+          meanSpeed += this->agents().at(agentId)->speed();
           ++n;
         }
       }
@@ -73,7 +73,7 @@ namespace dsm {
     if (node->isIntersection()) {
       auto& intersection = dynamic_cast<Intersection&>(*node);
       for (const auto& [angle, agentId] : intersection.agents()) {
-        const auto& agent{this->m_agents.at(agentId)};
+        const auto& agent{this->agents().at(agentId)};
         if (agent->streetId().has_value() && agent->streetId().value() == streetId) {
           meanSpeed += agent->speed();
           ++n;
@@ -82,7 +82,7 @@ namespace dsm {
     } else if (node->isRoundabout()) {
       auto& roundabout = dynamic_cast<Roundabout&>(*node);
       for (const auto& agentId : roundabout.agents()) {
-        const auto& agent{this->m_agents.at(agentId)};
+        const auto& agent{this->agents().at(agentId)};
         if (agent->streetId().has_value() && agent->streetId().value() == streetId) {
           meanSpeed += agent->speed();
           ++n;
@@ -93,7 +93,7 @@ namespace dsm {
   }
 
   Measurement<double> FirstOrderDynamics::streetMeanSpeed() const {
-    if (this->m_agents.size() == 0) {
+    if (this->agents().empty()) {
       return Measurement(0., 0.);
     }
     std::vector<double> speeds;
@@ -105,7 +105,7 @@ namespace dsm {
   }
   Measurement<double> FirstOrderDynamics::streetMeanSpeed(double threshold,
                                                           bool above) const {
-    if (this->m_agents.size() == 0) {
+    if (this->agents().empty()) {
       return Measurement(0., 0.);
     }
     std::vector<double> speeds;

src/dsm/headers/Graph.cpp

@@ -52,9 +52,8 @@ namespace dsm {
         throw std::invalid_argument(buildLog("Street with same id already exists."));
       }
       if (street->isSpire()) {
-        m_streets.emplace(
-            newStreetId,
-            std::make_unique<SpireStreet>(SpireStreet{newStreetId, *street}));
+        m_streets.emplace(newStreetId,
+                          std::make_unique<SpireStreet>(newStreetId, *street));
       } else {
         m_streets.emplace(newStreetId,
                           std::make_unique<Street>(Street{newStreetId, *street}));

src/dsm/headers/RoadDynamics.hpp

@@ -196,7 +196,7 @@ namespace dsm {
   Id RoadDynamics<delay_t>::m_nextStreetId(Id agentId,
                                            Id nodeId,
                                            std::optional<Id> streetId) {
-    auto const& pAgent{this->m_agents[agentId]};
+    auto const& pAgent{this->agents().at(agentId)};
     auto possibleMoves = this->m_graph.adjMatrix().getRow(nodeId, true);
     if (!pAgent->isRandom()) {
       std::uniform_real_distribution<double> uniformDist{0., 1.};
@@ -253,7 +253,7 @@ namespace dsm {
         continue;
       }
       const auto agentId{pStreet->queue(queueIndex).front()};
-      auto const& pAgent{this->m_agents[agentId]};
+      auto const& pAgent{this->agents().at(agentId)};
       if (pAgent->delay() > 0) {
         continue;
       }
@@ -331,10 +331,10 @@ namespace dsm {
           continue;
         }
         intersection.removeAgent(agentId);
-        this->m_agents[agentId]->setStreetId(nextStreet->id());
+        this->agents().at(agentId)->setStreetId(nextStreet->id());
         this->setAgentSpeed(agentId);
-        this->m_agents[agentId]->incrementDelay(
-            std::ceil(nextStreet->length() / this->m_agents[agentId]->speed()));
+        this->agents().at(agentId)->incrementDelay(
+            std::ceil(nextStreet->length() / this->agents().at(agentId)->speed()));
         nextStreet->addAgent(agentId);
         m_agentNextStreetId.erase(agentId);
         return true;
@@ -348,8 +348,8 @@ namespace dsm {
       auto const agentId{roundabout.agents().front()};
       auto const& nextStreet{this->m_graph.streetSet()[m_agentNextStreetId[agentId]]};
       if (!(nextStreet->isFull())) {
-        if (this->m_agents[agentId]->streetId().has_value()) {
-          const auto streetId = this->m_agents[agentId]->streetId().value();
+        if (this->agents().at(agentId)->streetId().has_value()) {
+          const auto streetId = this->agents().at(agentId)->streetId().value();
           auto delta = nextStreet->angle() - this->m_graph.streetSet()[streetId]->angle();
           if (delta > std::numbers::pi) {
             delta -= 2 * std::numbers::pi;
@@ -359,10 +359,10 @@ namespace dsm {
           m_increaseTurnCounts(streetId, delta);
         }
         roundabout.dequeue();
-        this->m_agents[agentId]->setStreetId(nextStreet->id());
+        this->agents().at(agentId)->setStreetId(nextStreet->id());
         this->setAgentSpeed(agentId);
-        this->m_agents[agentId]->incrementDelay(
-            std::ceil(nextStreet->length() / this->m_agents[agentId]->speed()));
+        this->agents().at(agentId)->incrementDelay(
+            std::ceil(nextStreet->length() / this->agents().at(agentId)->speed()));
         nextStreet->addAgent(agentId);
         m_agentNextStreetId.erase(agentId);
       } else {
@@ -377,7 +377,7 @@ namespace dsm {
   void RoadDynamics<delay_t>::m_evolveAgents() {
     std::uniform_int_distribution<Id> nodeDist{
         0, static_cast<Id>(this->m_graph.nNodes() - 1)};
-    for (const auto& [agentId, agent] : this->m_agents) {
+    for (const auto& [agentId, agent] : this->agents()) {
       if (agent->delay() > 0) {
         const auto& street{this->m_graph.streetSet()[agent->streetId().value()]};
         if (agent->delay() > 1) {
@@ -517,8 +517,8 @@ namespace dsm {
         itineraryId = itineraryIt->first;
       }
       Id agentId{0};
-      if (!this->m_agents.empty()) {
-        agentId = this->m_agents.rbegin()->first + 1;
+      if (!this->agents().empty()) {
+        agentId = this->agents().rbegin()->first + 1;
       }
       Id streetId{0};
       do {
@@ -527,13 +527,13 @@ namespace dsm {
         std::advance(streetIt, step);
         streetId = streetIt->first;
       } while (this->m_graph.streetSet()[streetId]->isFull() &&
-               this->m_agents.size() < this->m_graph.maxCapacity());
+               this->nAgents() < this->m_graph.maxCapacity());
       const auto& street{this->m_graph.streetSet()[streetId]};
       this->addAgent(agentId, itineraryId, street->nodePair().first);
-      this->m_agents[agentId]->setStreetId(streetId);
+      this->agents().at(agentId)->setStreetId(streetId);
       this->setAgentSpeed(agentId);
-      this->m_agents[agentId]->incrementDelay(
-          std::ceil(street->length() / this->m_agents[agentId]->speed()));
+      this->agents().at(agentId)->incrementDelay(
+          std::ceil(street->length() / this->agents().at(agentId)->speed()));
       street->addAgent(agentId);
       ++agentId;
     }
@@ -579,8 +579,8 @@ namespace dsm {
     std::uniform_real_distribution<double> srcUniformDist{0., srcSum};
     std::uniform_real_distribution<double> dstUniformDist{0., dstSum};
     Id agentId{0};
-    if (!this->m_agents.empty()) {
-      agentId = this->m_agents.rbegin()->first + 1;
+    if (!this->agents().empty()) {
+      agentId = this->agents().rbegin()->first + 1;
     }
     while (nAgents > 0) {
       Id srcId{0}, dstId{0};

src/dsm/headers/Street.hpp

@@ -68,6 +68,7 @@ namespace dsm {
            std::string name = std::string(),
            std::optional<int> capacity = std::nullopt,
            int transportCapacity = 1);
+    virtual ~Street() = default;
 
     /// @brief Set the street's queue
     /// @param queue The street's queue

test/CMakeLists.txt

@@ -8,10 +8,12 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
 
 # Add code coverage options
-if(CMAKE_BUILD_TYPE STREQUAL "Debug")
+if(CMAKE_BUILD_TYPE STREQUAL "COVERAGE")
     message(STATUS "Enable code coverage")
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -g --coverage -fprofile-update=atomic")
     set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} --coverage")
+else()
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -g -fsanitize=address")
 endif()
 
 # Set the folder for the executable

test/Test_dynamics.cpp

@@ -645,7 +645,6 @@ TEST_CASE("Dynamics") {
       graph2.buildAdj();
       graph2.adjustNodeCapacities();
       auto const& nodes = graph2.nodeSet();
-      auto& tl = dynamic_cast<TrafficLight&>(*nodes.at(1));
       nodes.at(0)->setCoords({0., -1.});
       nodes.at(2)->setCoords({0., 1.});
       nodes.at(3)->setCoords({-1., 0.});
@@ -715,7 +714,6 @@ TEST_CASE("Dynamics") {
       graph2.buildAdj();
       graph2.adjustNodeCapacities();
       auto const& nodes = graph2.nodeSet();
-      auto& tl = dynamic_cast<TrafficLight&>(*nodes.at(1));
       nodes.at(0)->setCoords({0., -1.});
       nodes.at(2)->setCoords({0., 1.});
       nodes.at(3)->setCoords({-1., 0.});
@@ -781,7 +779,6 @@ TEST_CASE("Dynamics") {
         Dynamics dynamics{graph2, 69};
         std::vector<dsm::Id> destinationNodes{0, 2, 3, 4};
         dynamics.setDestinationNodes(destinationNodes);
-        auto const& cycles{tl.cycles()};
         WHEN("We evolve the dynamics and optimize traffic lights") {
           dynamics.addAgents(7, 0, 2);
           dynamics.addAgents(7, 2, 0);
@@ -923,7 +920,6 @@ TEST_CASE("Dynamics") {
     CHECK_EQ(dynamics.streetMeanSpeed(0.2, true).std, 0.);
     CHECK_EQ(dynamics.streetMeanSpeed(0.2, false).mean, 15.);
     CHECK_EQ(dynamics.streetMeanSpeed(0.2, false).std, 0.);
-    (10, 0, 0);
     dynamics.evolve(false);
     meanSpeed = 0.;
     for (const auto& [agentId, agent] : dynamics.agents()) {
@@ -940,19 +936,19 @@ TEST_CASE("Dynamics") {
   SUBCASE("Intersection priorities") {
     GIVEN("A dynamics object with five nodes and eight streets") {
       Graph graph2;
-      auto& nodeO = graph2.addNode<Intersection>(0, std::make_pair(0, 0));
-      auto& nodeA = graph2.addNode<Intersection>(1, std::make_pair(-1, 1));
-      auto& nodeB = graph2.addNode<Intersection>(2, std::make_pair(1, 1));
-      auto& nodeC = graph2.addNode<Intersection>(3, std::make_pair(1, -1));
-      auto& nodeD = graph2.addNode<Intersection>(4, std::make_pair(-1, -1));
-      auto& sOA = graph2.addEdge<Street>(0, std::make_pair(0, 1), 10., 10.);
-      auto& sOB = graph2.addEdge<Street>(1, std::make_pair(0, 2), 10., 10.);
-      auto& sOC = graph2.addEdge<Street>(2, std::make_pair(0, 3), 10., 10.);
-      auto& sOD = graph2.addEdge<Street>(3, std::make_pair(0, 4), 10., 10.);
-      auto& sAO = graph2.addEdge<Street>(4, std::make_pair(1, 0), 10., 10.);
-      auto& sBO = graph2.addEdge<Street>(5, std::make_pair(2, 0), 10., 10.);
-      auto& sCO = graph2.addEdge<Street>(6, std::make_pair(3, 0), 10., 10.);
-      auto& sDO = graph2.addEdge<Street>(7, std::make_pair(4, 0), 10., 10.);
+      graph2.addNode<Intersection>(0, std::make_pair(0, 0));
+      graph2.addNode<Intersection>(1, std::make_pair(-1, 1));
+      graph2.addNode<Intersection>(2, std::make_pair(1, 1));
+      graph2.addNode<Intersection>(3, std::make_pair(1, -1));
+      graph2.addNode<Intersection>(4, std::make_pair(-1, -1));
+      graph2.addEdge<Street>(0, std::make_pair(0, 1), 10., 10.);
+      graph2.addEdge<Street>(1, std::make_pair(0, 2), 10., 10.);
+      graph2.addEdge<Street>(2, std::make_pair(0, 3), 10., 10.);
+      graph2.addEdge<Street>(3, std::make_pair(0, 4), 10., 10.);
+      graph2.addEdge<Street>(4, std::make_pair(1, 0), 10., 10.);
+      graph2.addEdge<Street>(5, std::make_pair(2, 0), 10., 10.);
+      graph2.addEdge<Street>(6, std::make_pair(3, 0), 10., 10.);
+      graph2.addEdge<Street>(7, std::make_pair(4, 0), 10., 10.);
       graph2.buildAdj();
       Dynamics dynamics{graph2, 69};
       dynamics.graph().nodeSet().at(0)->setCapacity(3);