fix: heap memory leak on SerializerFactory::OnHeap (#561)

* fix: Heap memory leak on serializer in ServiceProxy and Deserializer in TracingEchoOnConnection * fix: change SharedPtr to R value SharedPtr on EchoOnConnection::StartingMethod, release SharedPtrs' on TracingEchoOnConnection * fix: CRLF line ending in TracingEchoOnConnection * Update protobuf/echo/Echo.cpp Co-authored-by: Ron <45816308+rjaegers@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Richard Peters <richard.peters@philips.com> --------- Co-authored-by: Ron <45816308+rjaegers@users.noreply.github.com> Co-authored-by: Richard Peters <richard.peters@philips.com>
philips-software · Feb 20, 2024 · 0ed606b · 0ed606b
1 parent 363a701
commit 0ed606b
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 10 deletions.
diff --git a/protobuf/echo/Echo.cpp b/protobuf/echo/Echo.cpp
@@ -37,7 +37,7 @@ namespace services
     infra::SharedPtr<MethodSerializer> ServiceProxy::GrantSend()
     {
         onGranted();
-        return methodSerializer;
+        return std::move(methodSerializer);
     }
 
     uint32_t ServiceProxy::MaxMessageSize() const
@@ -74,7 +74,7 @@ namespace services
 
     void EchoOnStreams::ServiceDone()
     {
-        methodDeserializer = nullptr;
+        ReleaseDeserializer();
 
         if (readerPtr != nullptr)
             DataReceived();
@@ -116,9 +116,9 @@ namespace services
         return proxy.GrantSend();
     }
 
-    infra::SharedPtr<MethodDeserializer> EchoOnStreams::StartingMethod(uint32_t serviceId, uint32_t methodId, uint32_t size, const infra::SharedPtr<MethodDeserializer>& deserializer)
+    infra::SharedPtr<MethodDeserializer> EchoOnStreams::StartingMethod(uint32_t serviceId, uint32_t methodId, uint32_t size, infra::SharedPtr<MethodDeserializer>&& deserializer)
     {
-        return deserializer;
+        return std::move(deserializer);
     }
 
     void EchoOnStreams::SendStreamAvailable(infra::SharedPtr<infra::StreamWriter>&& writer)
@@ -236,10 +236,15 @@ namespace services
             if (methodDeserializer->Failed())
             {
                 errorPolicy.MessageFormatError();
-                methodDeserializer = nullptr;
+                ReleaseDeserializer();
             }
             else
                 methodDeserializer->ExecuteMethod();
         }
     }
+
+    void EchoOnStreams::ReleaseDeserializer()
+    {
+        methodDeserializer = nullptr;
+    }
 }
diff --git a/protobuf/echo/Echo.hpp b/protobuf/echo/Echo.hpp
@@ -78,13 +78,14 @@ namespace services
 
     protected:
         virtual infra::SharedPtr<MethodSerializer> GrantSend(ServiceProxy& proxy);
-        virtual infra::SharedPtr<MethodDeserializer> StartingMethod(uint32_t serviceId, uint32_t methodId, uint32_t size, const infra::SharedPtr<MethodDeserializer>& deserializer);
+        virtual infra::SharedPtr<MethodDeserializer> StartingMethod(uint32_t serviceId, uint32_t methodId, uint32_t size, infra::SharedPtr<MethodDeserializer>&& deserializer);
         virtual void RequestSendStream(std::size_t size) = 0;
         virtual void AckReceived() = 0;
 
         void SendStreamAvailable(infra::SharedPtr<infra::StreamWriter>&& writer);
         void DataReceived(infra::SharedPtr<infra::StreamReaderWithRewinding>&& reader);
         void ReleaseReader();
+        virtual void ReleaseDeserializer();
 
     private:
         void TryGrantSend();

diff --git a/services/network/TracingEchoOnConnection.cpp b/services/network/TracingEchoOnConnection.cpp
@@ -1,5 +1,7 @@
 #include "services/network/TracingEchoOnConnection.hpp"
 #include "infra/stream/BoundedVectorInputStream.hpp"
+#include "protobuf/echo/Echo.hpp"
+#include <cstddef>
 
 namespace services
 {
@@ -25,7 +27,7 @@ namespace services
         return infra::MakeContainedSharedObject(static_cast<MethodSerializer&>(*this), serializer);
     }
 
-    infra::SharedPtr<MethodDeserializer> TracingEchoOnConnection::StartingMethod(uint32_t serviceId, uint32_t methodId, uint32_t size, const infra::SharedPtr<MethodDeserializer>& deserializer)
+    infra::SharedPtr<MethodDeserializer> TracingEchoOnConnection::StartingMethod(uint32_t serviceId, uint32_t methodId, uint32_t size, infra::SharedPtr<MethodDeserializer>&& deserializer)
     {
         receivingService = FindService(serviceId);
 
@@ -34,8 +36,8 @@ namespace services
             receivingMethodId = methodId;
 
             readerBuffer.clear();
-            this->deserializer = deserializer;
-            return infra::MakeContainedSharedObject(static_cast<MethodDeserializer&>(*this), deserializer);
+            this->deserializer = std::move(deserializer);
+            return infra::MakeContainedSharedObject(static_cast<MethodDeserializer&>(*this), this->deserializer);
         }
         else
         {
@@ -44,6 +46,12 @@ namespace services
         }
     }
 
+    void TracingEchoOnConnection::ReleaseDeserializer()
+    {
+        EchoOnStreams::ReleaseDeserializer();
+        deserializer = nullptr;
+    }
+
     bool TracingEchoOnConnection::Serialize(infra::SharedPtr<infra::StreamWriter>&& writer)
     {
         return serializer->Serialize(tracingWriter.Emplace(std::move(writer), *this));
@@ -66,6 +74,7 @@ namespace services
             if (stream.Failed() || formatErrorPolicy.Failed())
                 tracer.Continue() << "... Malformed message";
         }
+        serializer = nullptr;
     }
 
     void TracingEchoOnConnection::MethodContents(infra::SharedPtr<infra::StreamReaderWithRewinding>&& reader)

diff --git a/services/network/TracingEchoOnConnection.hpp b/services/network/TracingEchoOnConnection.hpp
@@ -20,7 +20,8 @@ namespace services
     protected:
         // Implementation of EchoOnConnection
         infra::SharedPtr<MethodSerializer> GrantSend(ServiceProxy& proxy) override;
-        infra::SharedPtr<MethodDeserializer> StartingMethod(uint32_t serviceId, uint32_t methodId, uint32_t size, const infra::SharedPtr<MethodDeserializer>& deserializer) override;
+        infra::SharedPtr<MethodDeserializer> StartingMethod(uint32_t serviceId, uint32_t methodId, uint32_t size, infra::SharedPtr<MethodDeserializer>&& deserializer) override;
+        void ReleaseDeserializer() override;
 
     private:
         // Implementation of MethodSerializer