Version 1.2.1 alexa-client-sdk

Changes in this update
* **Enhancements**..
  * Added comments to `AlexaClientSDKConfig.json`. These descriptions provide additional guidance for what is expected for each field...
  * Enabled pause and resume controls for Pandora...

* **Bug Fixes**..
  * Bug fix for [issue alexa#329](alexa#329) - `HTTP2Transport` instances no longer leak when `SERVER_SIDE
  * Bug fix for [issue alexa#189](alexa#189) - Fixed a race condition in the `Timer` class that sometimes
  * Bug fix for a race condition that caused `SpeechSynthesizer` to ignore subsequent `Speak` directives...
  * Bug fix for corrupted mime attachments.

ACL/include/ACL/Transport/HTTP2Transport.h

@@ -363,6 +363,9 @@ class HTTP2Transport
     /// Whether or not the @c networkLoop is stopping. Serialized by @c m_mutex.
     bool m_isStopping;
 
+    /// Whether or not the onDisconnected() notification has been sent. Serialized by @c m_mutex.
+    bool m_disconnectedSent;
+
     /// Queue of @c MessageRequest instances to send. Serialized by @c m_mutex.
     std::deque<std::shared_ptr<avsCommon::avs::MessageRequest>> m_requestQueue;
 

ACL/include/ACL/Transport/MessageRouter.h

@@ -74,11 +74,13 @@ class MessageRouter
 
     void setObserver(std::shared_ptr<MessageRouterObserverInterface> observer) override;
 
-    void onConnected() override;
+    void onConnected(std::shared_ptr<TransportInterface> transport) override;
 
-    void onDisconnected(avsCommon::sdkInterfaces::ConnectionStatusObserverInterface::ChangedReason reason) override;
+    void onDisconnected(
+        std::shared_ptr<TransportInterface> transport,
+        avsCommon::sdkInterfaces::ConnectionStatusObserverInterface::ChangedReason reason) override;
 
-    void onServerSideDisconnect() override;
+    void onServerSideDisconnect(std::shared_ptr<TransportInterface> transport) override;
 
     void consumeMessage(const std::string& contextId, const std::string& message) override;
 

ACL/include/ACL/Transport/PostConnectSynchronizer.h

@@ -60,9 +60,11 @@ class PostConnectSynchronizer
     void onContextAvailable(const std::string& jsonContext) override;
     void onContextFailure(const avsCommon::sdkInterfaces::ContextRequestError error) override;
 
-    void onServerSideDisconnect() override;
-    void onConnected() override;
-    void onDisconnected(avsCommon::sdkInterfaces::ConnectionStatusObserverInterface::ChangedReason reason) override;
+    void onServerSideDisconnect(std::shared_ptr<TransportInterface> transport) override;
+    void onConnected(std::shared_ptr<TransportInterface> transport) override;
+    void onDisconnected(
+        std::shared_ptr<TransportInterface> transport,
+        avsCommon::sdkInterfaces::ConnectionStatusObserverInterface::ChangedReason reason) override;
 
     void addObserver(std::shared_ptr<PostConnectObserverInterface> observer) override;
     void removeObserver(std::shared_ptr<PostConnectObserverInterface> observer) override;

ACL/include/ACL/Transport/TransportObserverInterface.h

@@ -36,19 +36,27 @@ class TransportObserverInterface {
 
     /**
      * Called when a connection to AVS is established.
+     *
+     * @param transport The transport that has connected.
      */
-    virtual void onConnected() = 0;
+    virtual void onConnected(std::shared_ptr<TransportInterface> transport) = 0;
 
     /**
      * Called when we disconnect from AVS.
+     *
+     * @param transport The transport that is no longer connected (or attempting to connect).
      * @param reason The reason that we disconnected.
      */
-    virtual void onDisconnected(avsCommon::sdkInterfaces::ConnectionStatusObserverInterface::ChangedReason reason) = 0;
+    virtual void onDisconnected(
+        std::shared_ptr<TransportInterface> transport,
+        avsCommon::sdkInterfaces::ConnectionStatusObserverInterface::ChangedReason reason) = 0;
 
     /**
      * Called when the server asks the client to reconnect
+     *
+     * @param transport The transport that has received the disconnect request.
      */
-    virtual void onServerSideDisconnect() = 0;
+    virtual void onServerSideDisconnect(std::shared_ptr<TransportInterface> transport) = 0;
 };
 
 }  // namespace acl

ACL/src/Transport/HTTP2Transport.cpp

@@ -169,6 +169,7 @@ HTTP2Transport::HTTP2Transport(
         m_isNetworkThreadRunning{false},
         m_isConnected{false},
         m_isStopping{false},
+        m_disconnectedSent{false},
         m_postConnectObject{postConnectObject} {
     m_observers.insert(observer);
 
@@ -700,9 +701,10 @@ void HTTP2Transport::setIsConnectedFalse() {
     auto disconnectReason = ConnectionStatusObserverInterface::ChangedReason::INTERNAL_ERROR;
     {
         std::lock_guard<std::mutex> lock(m_mutex);
-        if (!m_isConnected) {
+        if (m_disconnectedSent) {
             return;
         }
+        m_disconnectedSent = true;
         m_isConnected = false;
         disconnectReason = m_disconnectReason;
     }
@@ -775,7 +777,7 @@ void HTTP2Transport::notifyObserversOnServerSideDisconnect() {
     lock.unlock();
 
     for (auto observer : observers) {
-        observer->onServerSideDisconnect();
+        observer->onServerSideDisconnect(shared_from_this());
     }
 }
 
@@ -785,7 +787,7 @@ void HTTP2Transport::notifyObserversOnDisconnect(ConnectionStatusObserverInterfa
     lock.unlock();
 
     for (auto observer : observers) {
-        observer->onDisconnected(reason);
+        observer->onDisconnected(shared_from_this(), reason);
     }
 }
 
@@ -795,7 +797,7 @@ void HTTP2Transport::notifyObserversOnConnected() {
     lock.unlock();
 
     for (auto observer : observers) {
-        observer->onConnected();
+        observer->onConnected(shared_from_this());
     }
 }
 

ACL/src/Transport/MessageRouter.cpp

@@ -112,7 +112,7 @@ void MessageRouter::setAVSEndpoint(const std::string& avsEndpoint) {
     }
 }
 
-void MessageRouter::onConnected() {
+void MessageRouter::onConnected(std::shared_ptr<TransportInterface> transport) {
     std::unique_lock<std::mutex> lock{m_connectionMutex};
     if (m_isEnabled) {
         setConnectionStatusLocked(
@@ -121,45 +121,34 @@ void MessageRouter::onConnected() {
     }
 }
 
-void MessageRouter::onDisconnected(ConnectionStatusObserverInterface::ChangedReason reason) {
+void MessageRouter::onDisconnected(
+    std::shared_ptr<TransportInterface> transport,
+    ConnectionStatusObserverInterface::ChangedReason reason) {
     std::lock_guard<std::mutex> lock{m_connectionMutex};
-    if (ConnectionStatusObserverInterface::Status::CONNECTED == m_connectionStatus) {
-        // Reset m_activeTransport of it is not longer connected.
-        if (m_activeTransport && !m_activeTransport->isConnected()) {
-            safelyResetActiveTransportLocked();
-        }
-
-        // Trim m_transports to just those transports still connected.  Also build list of disconnected transports.
-        std::vector<std::shared_ptr<TransportInterface>> connected;
-        std::vector<std::shared_ptr<TransportInterface>> disconnected;
-        for (auto transport : m_transports) {
-            if (transport->isConnected()) {
-                connected.push_back(transport);
-            } else {
-                disconnected.push_back(transport);
-            }
-        }
-        m_transports.clear();
-        std::swap(m_transports, connected);
 
-        // Release all the disconnected transports.
-        for (auto transport : disconnected) {
+    for (auto it = m_transports.begin(); it != m_transports.end(); it++) {
+        if (*it == transport) {
+            m_transports.erase(it);
             safelyReleaseTransport(transport);
+            break;
         }
+    }
 
+    if (transport == m_activeTransport) {
+        m_activeTransport.reset();
         // Update status.  If transitioning to PENDING, also initiate the reconnect.
-        if (m_isEnabled) {
-            if (!m_activeTransport) {
+        if (ConnectionStatusObserverInterface::Status::CONNECTED == m_connectionStatus) {
+            if (m_isEnabled) {
                 setConnectionStatusLocked(ConnectionStatusObserverInterface::Status::PENDING, reason);
                 createActiveTransportLocked();
+            } else if (m_transports.empty()) {
+                setConnectionStatusLocked(ConnectionStatusObserverInterface::Status::DISCONNECTED, reason);
             }
-        } else if (m_transports.empty()) {
-            setConnectionStatusLocked(ConnectionStatusObserverInterface::Status::DISCONNECTED, reason);
         }
     }
 }
 
-void MessageRouter::onServerSideDisconnect() {
+void MessageRouter::onServerSideDisconnect(std::shared_ptr<TransportInterface> transport) {
     std::unique_lock<std::mutex> lock{m_connectionMutex};
     if (m_isEnabled) {
         setConnectionStatusLocked(

ACL/src/Transport/MimeParser.cpp

@@ -162,6 +162,11 @@ MimeParser::DataParsedStatus MimeParser::writeDataToAttachment(const char* buffe
 void MimeParser::partDataCallback(const char* buffer, size_t size, void* userData) {
     MimeParser* parser = static_cast<MimeParser*>(userData);
 
+    if (MimeParser::DataParsedStatus::INCOMPLETE == parser->m_dataParsedStatus) {
+        ACSDK_DEBUG9(LX("partDataCallbackIgnored").d("reason", "attachmentWriterFullBuffer"));
+        return;
+    }
+
     if (parser->m_dataParsedStatus != MimeParser::DataParsedStatus::OK) {
         ACSDK_ERROR(
             LX("partDataCallbackFailed").d("reason", "mimeParsingError").d("status", parser->m_dataParsedStatus));
@@ -170,7 +175,7 @@ void MimeParser::partDataCallback(const char* buffer, size_t size, void* userDat
 
     // If we've already processed any of this part in a previous incomplete iteration, let's not process it twice.
     if (!parser->shouldProcessBytes(size)) {
-        ACSDK_DEBUG(LX("partDataCallbackSkipped").d("reason", "bytesAlreadyProcessed"));
+        ACSDK_DEBUG9(LX("partDataCallbackSkipped").d("reason", "bytesAlreadyProcessed"));
         parser->updateCurrentByteProgress(size);
         parser->m_dataParsedStatus = MimeParser::DataParsedStatus::OK;
         return;
@@ -304,6 +309,7 @@ MimeParser::DataParsedStatus MimeParser::feed(char* data, size_t length) {
     }
 
     // Initialize this before all the feed() callbacks happen (since this persists from previous call).
+    m_currentByteProgress = 0;
     m_dataParsedStatus = DataParsedStatus::OK;
 
     m_multipartReader.feed(data, length);
@@ -350,7 +356,7 @@ void MimeParser::setAttachmentWriterBufferFull(bool isFull) {
     if (isFull == m_isAttachmentWriterBufferFull) {
         return;
     }
-    ACSDK_DEBUG0(LX("setAttachmentWriterBufferFull").d("full", isFull));
+    ACSDK_DEBUG9(LX("setAttachmentWriterBufferFull").d("full", isFull));
     m_isAttachmentWriterBufferFull = isFull;
 }
 

ACL/src/Transport/PostConnectSynchronizer.cpp

@@ -240,16 +240,18 @@ void PostConnectSynchronizer::notifyObservers() {
     }
 }
 
-void PostConnectSynchronizer::onServerSideDisconnect() {
+void PostConnectSynchronizer::onServerSideDisconnect(std::shared_ptr<TransportInterface> transport) {
     ACSDK_DEBUG(LX("onServerSideDisconnect()"));
     doShutdown();
 }
 
-void PostConnectSynchronizer::onConnected() {
+void PostConnectSynchronizer::onConnected(std::shared_ptr<TransportInterface> transport) {
     ACSDK_DEBUG(LX("onConnected()"));
 }
 
-void PostConnectSynchronizer::onDisconnected(ConnectionStatusObserverInterface::ChangedReason reason) {
+void PostConnectSynchronizer::onDisconnected(
+    std::shared_ptr<TransportInterface> transport,
+    ConnectionStatusObserverInterface::ChangedReason reason) {
     ACSDK_DEBUG(LX("onDisconnected()"));
     doShutdown();
 }

ACL/test/Transport/MessageRouterTest.cpp

@@ -39,7 +39,7 @@ TEST_F(MessageRouterTest, getConnectionStatusReturnsConnectedAfterConnectionEsta
 }
 
 TEST_F(MessageRouterTest, getConnectionStatusReturnsConnectedAfterDisconnected) {
-    m_router->onDisconnected(ConnectionStatusObserverInterface::ChangedReason::ACL_DISABLED);
+    m_router->onDisconnected(m_mockTransport, ConnectionStatusObserverInterface::ChangedReason::ACL_DISABLED);
     ASSERT_EQ(m_router->getConnectionStatus().first, ConnectionStatusObserverInterface::Status::DISCONNECTED);
 }
 
@@ -74,7 +74,7 @@ TEST_F(MessageRouterTest, ensureTheMessageRouterObserverIsInformedOfTransportDis
 
     auto reason = ConnectionStatusObserverInterface::ChangedReason::ACL_DISABLED;
     disconnectMockTransport(m_mockTransport.get());
-    m_router->onDisconnected(reason);
+    m_router->onDisconnected(m_mockTransport, reason);
 
     // wait for the result to propagate by scheduling a task on the client executor
     waitOnMessageRouter(SHORT_TIMEOUT_MS);
@@ -179,7 +179,7 @@ TEST_F(MessageRouterTest, serverSideDisconnectCreatesANewTransport) {
     m_router->setMockTransport(newTransport);
 
     // Reset the MessageRouterObserver, there should be no interactions with the observer
-    m_router->onServerSideDisconnect();
+    m_router->onServerSideDisconnect(oldTransport);
 
     waitOnMessageRouter(SHORT_TIMEOUT_MS);
 
@@ -191,7 +191,7 @@ TEST_F(MessageRouterTest, serverSideDisconnectCreatesANewTransport) {
 
     // mock the new transports connection
     connectMockTransport(newTransport.get());
-    m_router->onConnected();
+    m_router->onConnected(newTransport);
 
     waitOnMessageRouter(SHORT_TIMEOUT_MS);
 
@@ -203,7 +203,7 @@ TEST_F(MessageRouterTest, serverSideDisconnectCreatesANewTransport) {
 
     // mock the old transport disconnecting completely
     disconnectMockTransport(oldTransport.get());
-    m_router->onDisconnected(ConnectionStatusObserverInterface::ChangedReason::ACL_CLIENT_REQUEST);
+    m_router->onDisconnected(oldTransport, ConnectionStatusObserverInterface::ChangedReason::ACL_CLIENT_REQUEST);
 
     auto messageRequest = createMessageRequest();
 

ACL/test/Transport/MessageRouterTest.h

@@ -120,7 +120,7 @@ class MessageRouterTest : public ::testing::Test {
 
     void setupStateToConnected() {
         setupStateToPending();
-        m_router->onConnected();
+        m_router->onConnected(m_mockTransport);
         connectMockTransport(m_mockTransport.get());
     }