fix: sesame/echo receive deadlock on sliced messages

services/util/SesameSecured.cpp

@@ -171,7 +171,16 @@ namespace services
 
         IncreaseIv(receiveIv);
 
-        Sesame::GetObserver().ReceivedMessage(receiveBufferReader.Emplace(receiveBuffer, reader));
+        receivedReader = std::move(reader);
+        Sesame::GetObserver().ReceivedMessage(receiveBufferReader.Emplace(receiveBuffer));
+    }
+
+    void SesameSecured::ReleaseReceivedReader()
+    {
+        // Releasing the reader lets the lower layer forward the next message into receiveBufferReader.
+        // Move it out first, so that a re-entrant ReceivedMessage() assigns to an empty receivedReader
+        // instead of having its assignment overwritten while this SharedPtr is being reset.
+        auto reader = std::move(receivedReader);
     }
 
     void SesameSecured::SendMessageStreamReleased()
@@ -204,11 +213,6 @@ namespace services
             });
     }
 
-    SesameSecured::ReceiveBufferReader::ReceiveBufferReader(const infra::BoundedVector<uint8_t>& buffer, const infra::SharedPtr<infra::StreamReaderWithRewinding>& reader)
-        : infra::BoundedVectorInputStreamReader(buffer)
-        , reader(reader)
-    {}
-
 #ifdef EMIL_USE_MBEDTLS
     SesameSecured::WithCryptoMbedTls::WithCryptoMbedTls(infra::BoundedVector<uint8_t>& sendBuffer, infra::BoundedVector<uint8_t>& receiveBuffer, Sesame& delegate, const KeyMaterial& keyMaterial)
         : SesameSecured(detail::SesameSecuredMbedTlsEncryptors::sendEncryption, detail::SesameSecuredMbedTlsEncryptors::receiveEncryption, sendBuffer, receiveBuffer, delegate, keyMaterial)

services/util/SesameSecured.hpp

@@ -82,20 +82,10 @@ namespace services
 
         void ActivateSendKey();
         void SendMessageStreamReleased();
+        void ReleaseReceivedReader();
         void IncreaseIv(infra::ByteRange iv) const;
         void ReportIntegrityCheckFailed();
 
-    private:
-        class ReceiveBufferReader
-            : public infra::BoundedVectorInputStreamReader
-        {
-        public:
-            ReceiveBufferReader(const infra::BoundedVector<uint8_t>& buffer, const infra::SharedPtr<infra::StreamReaderWithRewinding>& reader);
-
-        private:
-            infra::SharedPtr<infra::StreamReaderWithRewinding> reader;
-        };
-
     private:
         AesGcmEncryption& sendEncryption;
         AesGcmEncryption& receiveEncryption;
@@ -114,7 +104,11 @@ namespace services
         std::array<uint8_t, keySize> initialReceiveKey;
         std::array<uint8_t, ivSize> initialReceiveIv;
         std::array<uint8_t, ivSize> receiveIv;
-        infra::SharedOptional<ReceiveBufferReader> receiveBufferReader;
+        infra::SharedPtr<infra::StreamReaderWithRewinding> receivedReader;
+        infra::NotifyingSharedOptional<infra::BoundedVectorInputStreamReader> receiveBufferReader{ [this]()
+            {
+                ReleaseReceivedReader();
+            } };
         bool integrityCheckFailed = false;
         infra::TimerSingleShot integrityCheckFailedTimer;
     };

services/util/test/TestEchoInstantiationSecured.cpp

@@ -85,6 +85,59 @@ TEST_F(EchoInstantiationSecuredSymmetricKeyTest, send_multiple_messages)
     ExecuteAllActions();
 }
 
+class EchoInstantiationSecuredSymmetricKeySlicedTest
+    : public testing::Test
+    , public infra::ClockFixture
+    , public EchoInstantiationSecuredSymmetricKey<128, 128>
+{
+public:
+    EchoInstantiationSecuredSymmetricKeySlicedTest()
+    {
+        payload.resize(payload.max_size());
+    }
+
+    void SendSlicedMessage(services::ServiceStubProxy& proxy)
+    {
+        proxy.RequestSend([this, &proxy]()
+            {
+                proxy.MethodBytes(infra::MakeRange(payload));
+            });
+    }
+
+    infra::BoundedVector<uint8_t>::WithMaxSize<64> payload;
+    std::array<services::ServiceStubProxy, 4> slicedMessageProxies{ services::ServiceStubProxy{ leftEcho.echo }, services::ServiceStubProxy{ leftEcho.echo },
+        services::ServiceStubProxy{ leftEcho.echo }, services::ServiceStubProxy{ leftEcho.echo } };
+};
+
+TEST_F(EchoInstantiationSecuredSymmetricKeySlicedTest, sliced_messages_are_received_while_a_method_is_still_executing)
+{
+    bool methodNoParameterInvoked = false;
+    EXPECT_CALL(service, MethodNoParameter()).WillOnce(testing::Invoke([&methodNoParameterInvoked]()
+        {
+            methodNoParameterInvoked = true;
+        }));
+    EXPECT_CALL(service, MethodBytes(testing::_)).Times(slicedMessageProxies.size()).WillRepeatedly(testing::Invoke([this](const infra::BoundedVector<uint8_t>& value)
+        {
+            EXPECT_EQ(payload, value);
+            service.MethodDone();
+        }));
+
+    serviceProxy.RequestSend([this]()
+        {
+            serviceProxy.MethodNoParameter();
+        });
+    for (auto& proxy : slicedMessageProxies)
+        SendSlicedMessage(proxy);
+
+    ExecuteAllActions();
+    ASSERT_TRUE(methodNoParameterInvoked);
+
+    // MethodNoParameter() completes only now, so the first slice of the next message has been
+    // received while the deserializer was still busy
+    service.MethodDone();
+    ExecuteAllActions();
+}
+
 namespace
 {
     template<std::size_t LeftSize, std::size_t RightSize>