[C++ Wrapper] Decrement ref count of an Image after it was created,…

… because it was counted twice: once in the C code when looking the `aeron_image_t` and the second time by invoking `aeron_subscription_image_retain` inside the `Image` constructor.
aeron-io · Feb 7, 2025 · 2593db3 · 2593db3
1 parent 6674ab9
commit 2593db3
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 26 deletions.
diff --git a/aeron-client/src/main/cpp_wrapper/Subscription.h b/aeron-client/src/main/cpp_wrapper/Subscription.h
@@ -492,7 +492,9 @@ class Subscription
             return nullptr;
         }
 
-        return std::make_shared<Image>(m_subscription, image);
+        auto result = std::make_shared<Image>(m_subscription, image);
+        aeron_subscription_image_release(m_subscription, image);
+        return result;
     }
 
     /**
@@ -512,7 +514,9 @@ class Subscription
             throw std::logic_error("index out of range");
         }
 
-        return std::make_shared<Image>(m_subscription, image);
+        auto result = std::make_shared<Image>(m_subscription, image);
+        aeron_subscription_image_release(m_subscription, image);
+        return result;
     }
 
     /**
@@ -569,26 +573,6 @@ class Subscription
     {
         return m_subscription;
     }
-
-    /// @cond HIDDEN_SYMBOLS
-    bool hasImage(std::int64_t correlationId) const
-    {
-        bool hasImage = false;
-        auto imageList = copyOfImageList();
-
-        for (auto &image : *imageList)
-        {
-            if (image->correlationId() == correlationId)
-            {
-                hasImage = true;
-                break;
-            }
-        }
-
-        return hasImage;
-    }
-    /// @endcond
-
 private:
     aeron_t *m_aeron = nullptr;
     aeron_subscription_t *m_subscription = nullptr;

diff --git a/aeron-client/src/test/cpp_wrapper/SystemTest.cpp b/aeron-client/src/test/cpp_wrapper/SystemTest.cpp
@@ -261,7 +261,7 @@ INSTANTIATE_TEST_SUITE_P(
     SystemTestParameterized,
     testing::Values("aeron:ipc?alias=test", "aeron:udp?alias=test|endpoint=localhost:8092"));
 
-TEST_P(SystemTestParameterized, DISABLED_shouldFreeUnavailableImage)
+TEST_P(SystemTestParameterized, shouldFreeUnavailableImage)
 {
     std::string channel = GetParam();
     const int stream_id = 1000;
@@ -300,6 +300,7 @@ TEST_P(SystemTestParameterized, DISABLED_shouldFreeUnavailableImage)
     }
     while (nullptr == subscription);
 
+    aeron_image_t *raw_image = nullptr;
     {
         std::shared_ptr<Image> image;
         do
@@ -319,12 +320,13 @@ TEST_P(SystemTestParameterized, DISABLED_shouldFreeUnavailableImage)
         EXPECT_NE(image, image_by_index);
         EXPECT_EQ(image_correlation_id, image_by_index->correlationId());
 
-        const auto raw_image =
+        raw_image =
             aeron_subscription_image_by_session_id(subscription->subscription(), publication->sessionId());
-        EXPECT_EQ(2, aeron_image_decr_refcnt(raw_image));
-        EXPECT_EQ(1, aeron_image_refcnt_volatile(raw_image));
+        EXPECT_EQ(4, aeron_image_decr_refcnt(raw_image));
+        EXPECT_EQ(3, aeron_image_refcnt_volatile(raw_image));
     }
 
+    EXPECT_EQ(1, aeron_image_refcnt_volatile(raw_image));
     EXPECT_EQ(1, subscription->imageCount());
     EXPECT_NE(nullptr, subscription->imageBySessionId(publication->sessionId()));