commit 883a8929e3fba37460895d41435b9ae286c5406c
author Benjamin Fair <benjaminfair@google.com> Wed Jan 22 22:07:23 2025 +0000
committer Willy Tu <wltu@google.com> Sat Jan 25 01:14:36 2025 +0000
tree 3beb26f82ced302b2a46014e31e1e601e024910a
parent d01fca35bf709c179ba5ade01b127b2a31ebfbce

associations: fix leak in assocMaps.ifaces

New entries get added to ifaces whenever an object with associations is
detected, but the entries are never removed even if all the endpoints
are gone, leading to a memory leak.

Change-Id: Ide5439e4eaab7c70fb0331d59d59753905f45066
Signed-off-by: Benjamin Fair <benjaminfair@google.com>
Signed-off-by: Willy Tu <wltu@google.com>

src/associations.cpp

diff --git a/src/associations.cpp b/src/associations.cpp
index 1899826..3293b4e 100644
--- a/src/associations.cpp
+++ b/src/associations.cpp
@@ -10,9 +10,14 @@
                            const std::string& assocPath,
                            AssociationMaps& assocMaps)
 {
-    auto& iface = assocMaps.ifaces[assocPath];
-    auto& i = std::get<ifacePos>(iface);
-    auto& endpoints = std::get<endpointsPos>(iface);
+    // Don't create an entry in assocMaps.ifaces if not needed.
+    auto iface = assocMaps.ifaces.find(assocPath);
+    if (iface == assocMaps.ifaces.end())
+    {
+        return;
+    }
+    auto& i = std::get<ifacePos>(iface->second);
+    auto& endpoints = std::get<endpointsPos>(iface->second);
 
     // If the interface already exists, only need to update
     // the property value, otherwise create it
@@ -28,14 +33,16 @@
             i->set_property("endpoints", endpoints);
         }
     }
-    else
+    else if (!endpoints.empty())
     {
-        if (!endpoints.empty())
-        {
-            i = objectServer.add_interface(assocPath, xyzAssociationInterface);
-            i->register_property("endpoints", endpoints);
-            i->initialize();
-        }
+        i = objectServer.add_interface(assocPath, xyzAssociationInterface);
+        i->register_property("endpoints", endpoints);
+        i->initialize();
+    }
+
+    if (endpoints.empty())
+    {
+        assocMaps.ifaces.erase(iface);
     }
 }
 
@@ -50,8 +57,12 @@
         return;
     }
 
-    auto& iface = assocMaps.ifaces[assocPath];
-    auto& endpoints = std::get<endpointsPos>(iface);
+    auto iface = assocMaps.ifaces.find(assocPath);
+    if (iface == assocMaps.ifaces.end())
+    {
+        return;
+    }
+    auto& endpoints = std::get<endpointsPos>(iface->second);
 
     if (endpoints.size() > endpointsCountTimerThreshold)
     {

src/test/associations.cpp

diff --git a/src/test/associations.cpp b/src/test/associations.cpp
index 62aefab..fe0a826 100644
--- a/src/test/associations.cpp
+++ b/src/test/associations.cpp
@@ -313,9 +313,9 @@
     associationChanged(io, *server, associations, defaultSourcePath,
                        defaultDbusSvc, interfaceMap, assocMaps);
 
-    // Should have 3 entries in AssociationInterfaces, one is just missing an
-    // endpoint
-    EXPECT_EQ(assocMaps.ifaces.size(), 3);
+    // Should have 2 entries in AssociationInterfaces
+    // The one missing an endpoint is not added.
+    EXPECT_EQ(assocMaps.ifaces.size(), 2);
 
     // Change to existing interface so it will be removed here
     auto intfEndpoints =