News, Unit Testing, Tweaks for Unit Testing

dds/DCPS/AddressCache.h

@@ -17,9 +17,6 @@
 #include "PoolAllocator.h"
 #include "RcObject.h"
 #include "TimeTypes.h"
-#include "dcps_export.h"
-
-#include <ace/INET_Addr.h>
 
 OPENDDS_BEGIN_VERSIONED_NAMESPACE_DECL
 
@@ -30,15 +27,18 @@ typedef OPENDDS_SET_CMP(GUID_t, GUID_tKeyLessThan) GuidSet;
 
 struct AddressCacheEntry : public virtual RcObject {
 
-  AddressCacheEntry() : addrs_(), expires_(MonotonicTimePoint::max_value)
+  AddressCacheEntry()
+    : expires_(MonotonicTimePoint::max_value)
 #if defined ACE_HAS_CPP11
     , addrs_hash_(0)
 #endif
   {}
 
-  AddressCacheEntry(const NetworkAddressSet& addrs, const MonotonicTimePoint& expires) : addrs_(addrs), expires_(expires)
+  AddressCacheEntry(const NetworkAddressSet& addrs, const MonotonicTimePoint& expires)
+    : addrs_(addrs)
+    , expires_(expires)
 #if defined ACE_HAS_CPP11
-  , addrs_hash_(calculate_hash(addrs_))
+    , addrs_hash_(calculate_hash(addrs_))
 #endif
   {}
 
@@ -209,7 +209,7 @@ class AddressCache {
   bool remove(const Key& key)
   {
     ACE_Guard<ACE_Thread_Mutex> guard(mutex_);
-    return map_.erase(key) != 0;
+    return remove_i(key);
   }
 
   void remove_id(const GUID_t& val)
@@ -223,23 +223,15 @@ class AddressCache {
     }
 
     for (typename KeySet::iterator key = keys.begin(), limit = keys.end(); key != limit; ++key) {
-      map_.erase(*key);
-
-      // Undo insert_ids(key)
-      GuidSet guids;
-      key->get_contained_guids(guids);
-      for (GuidSet::const_iterator guid = guids.begin(), limit = guids.end(); guid != limit; ++guid) {
-        const typename IdMapType::iterator other = id_map_.find(*guid);
-        if (other != id_map_.end()) {
-          other->second.erase(*key);
-          if (other->second.empty()) {
-            id_map_.erase(other);
-          }
-        }
-      }
+      remove_i(*key);
     }
   }
 
+  bool empty() const
+  {
+    return map_.empty() && id_map_.empty();
+  }
+
 private:
 
   void insert_ids(const Key& key)
@@ -251,6 +243,26 @@ class AddressCache {
     }
   }
 
+  bool remove_i(const Key& key)
+  {
+    const bool found = map_.erase(key) != 0;
+
+    // Undo insert_ids(key)
+    GuidSet guids;
+    key.get_contained_guids(guids);
+    for (GuidSet::const_iterator guid = guids.begin(), limit = guids.end(); guid != limit; ++guid) {
+      const typename IdMapType::iterator other = id_map_.find(*guid);
+      if (other != id_map_.end()) {
+        other->second.erase(key);
+        if (other->second.empty()) {
+          id_map_.erase(other);
+        }
+      }
+    }
+
+    return found;
+  }
+
   mutable ACE_Thread_Mutex mutex_;
   MapType map_;
   IdMapType id_map_;

docs/news.d/addr-cache-leak.rst

@@ -0,0 +1,5 @@
+.. news-prs: 4772
+
+.. news-start-section: Fixes
+- Fixed a memory leak in the address caches used by the RTPS/UDP transport.
+.. news-end-section

tests/unit-tests/dds/DCPS/AddressCache.cpp

@@ -81,27 +81,37 @@ TEST(dds_DCPS_AddressCache, store_remove_load_fail)
   AddressCache<TestKey> test_cache_;
   NetworkAddressSet addrs;
   addrs.insert(NetworkAddress("127.0.0.1:1234"));
+  const GUID_t a = {{1, 1, 1}, {{1, 1, 1}, 1}};
+  const GUID_t b = {{2, 2, 2}, {{2, 2, 2}, 2}};
+  const TestKey key(a, b);
 
-  test_cache_.store(TestKey(GUID_UNKNOWN, GUID_UNKNOWN), addrs);
+  test_cache_.store(key, addrs);
   addrs.clear();
 
-  test_cache_.remove(TestKey(GUID_UNKNOWN, GUID_UNKNOWN));
+  ASSERT_FALSE(test_cache_.empty());
+  test_cache_.remove(key);
+  ASSERT_TRUE(test_cache_.empty());
 
-  ASSERT_FALSE(test_cache_.load(TestKey(GUID_UNKNOWN, GUID_UNKNOWN), addrs));
+  ASSERT_FALSE(test_cache_.load(key, addrs));
 }
 
 TEST(dds_DCPS_AddressCache, store_remove_id_load_fail)
 {
   AddressCache<TestKey> test_cache_;
   NetworkAddressSet addrs;
   addrs.insert(NetworkAddress("127.0.0.1:1234"));
+  const GUID_t a = {{1, 1, 1}, {{1, 1, 1}, 1}};
+  const GUID_t b = {{2, 2, 2}, {{2, 2, 2}, 2}};
+  const TestKey key(a, b);
 
-  test_cache_.store(TestKey(GUID_UNKNOWN, GUID_UNKNOWN), addrs);
+  test_cache_.store(key, addrs);
   addrs.clear();
 
-  test_cache_.remove_id(GUID_UNKNOWN);
+  ASSERT_FALSE(test_cache_.empty());
+  test_cache_.remove_id(a);
+  ASSERT_TRUE(test_cache_.empty());
 
-  ASSERT_FALSE(test_cache_.load(TestKey(GUID_UNKNOWN, GUID_UNKNOWN), addrs));
+  ASSERT_FALSE(test_cache_.load(key, addrs));
 }
 
 TEST(dds_DCPS_AddressCache, scoped_access_load_success)