Version 1.0.1 alexa-client-sdk

Changes in this update
 - Added a fix to the sample app so that the StateSynchronization event is the first that gets sent to AVS.
 - Added a POST_CONNECTED enum to ConnectionStatusObserver.
 - StateSychronizer now automatically sends a StateSynchronization event when it receives a notification that ACL is CONNECTED.
 - Added make install for installing the AVS Device SDK.
 - Added an optional make networkIntegration for integration tests for slow network (only on Linux platforms).
 - Added shutdown management which fully cleans up SDK objects during teardown.
 - Fixed an issue with AudioPlayer barge-in which was preventing subsequent audio from playing.
 - Changed Mediaplayer buffering to reduce stuttering.
 - Known Issues:
   - Connection loss during listening keeps the app in that state even after connection is regained. Pressing ‘s’ unsticks the state.
   - Play/Pause media restarts it from the beginning.
   - SpeechSynthesizer shows wrong UX state during a burst of Speaks.
   - Quitting the sample app while AudioPlayer is playing something causes a segmentation fault.
   - AudioPlayer sending PlaybackPaused during flash briefing.
   - Long Delay playing live stations on iHeartRadio.
   - Teardown warnings at the end of integration tests.

ACL/include/ACL/AVSConnectionManager.h

@@ -30,6 +30,7 @@
 #include <AVSCommon/SDKInterfaces/MessageObserverInterface.h>
 #include <AVSCommon/SDKInterfaces/MessageSenderInterface.h>
 #include <AVSCommon/SDKInterfaces/StateSynchronizerObserverInterface.h>
+#include <AVSCommon/Utils/RequiresShutdown.h>
 
 #include "ACL/Transport/MessageRouterInterface.h"
 #include "ACL/Transport/MessageRouterObserverInterface.h"
@@ -72,7 +73,8 @@ class AVSConnectionManager :
     public avsCommon::sdkInterfaces::AVSEndpointAssignerInterface,
     /* TODO: ACSDK-421: Remove the implementation of StateSynchronizerObserverInterface */
     public avsCommon::sdkInterfaces::StateSynchronizerObserverInterface,
-    public MessageRouterObserverInterface {
+    public MessageRouterObserverInterface,
+    public avsCommon::utils::RequiresShutdown {
 public:
     /**
      * A factory function that creates an AVSConnectionManager object.
@@ -192,6 +194,8 @@ class AVSConnectionManager :
                     messageObserver = 
                     std::unordered_set<std::shared_ptr<avsCommon::sdkInterfaces::MessageObserverInterface>>());
 
+    void doShutdown() override;
+
     void onConnectionStatusChanged(
             const avsCommon::sdkInterfaces::ConnectionStatusObserverInterface::Status status,
             const avsCommon::sdkInterfaces::ConnectionStatusObserverInterface::ChangedReason reason) override;
@@ -203,7 +207,10 @@ class AVSConnectionManager :
 
     /* TODO: ACSDK-421: Remove the implementation of StateSynchronizerObserverInterface */
     /// Internal object that flags if @c StateSynchronizer had sent the initial event successfully.
-    std::atomic<bool> m_isSynchronized;
+    bool m_isSynchronized;
+
+    /// Mutex to protect @c m_isSynchronized.
+    std::mutex m_synchronizationMutex;
 
     /// Set of observers to notify when the connection status changes. @c m_connectionStatusObserverMutex must be
     /// acquired before access.

ACL/src/AVSConnectionManager.cpp

@@ -82,13 +82,27 @@ AVSConnectionManager::AVSConnectionManager(
         std::shared_ptr<MessageRouterInterface> messageRouter,
         std::unordered_set<std::shared_ptr<ConnectionStatusObserverInterface>> connectionStatusObservers,
         std::unordered_set<std::shared_ptr<MessageObserverInterface>> messageObservers)
-        : m_isEnabled{false},
+        : RequiresShutdown{"AVSConnectionManager"},
+          m_isEnabled{false},
           m_isSynchronized{false},
           m_connectionStatusObservers{connectionStatusObservers},
           m_messageObservers{messageObservers},
           m_messageRouter{messageRouter} {
 }
 
+void AVSConnectionManager::doShutdown() {
+    disable();
+    {
+        std::lock_guard<std::mutex> lock{m_connectionStatusObserverMutex};
+        m_connectionStatusObservers.clear();
+    }
+    {
+        std::lock_guard<std::mutex> lock{m_messageOberverMutex};
+        m_messageObservers.clear();
+    }
+    m_messageRouter.reset();
+}
+
 void AVSConnectionManager::enable() {
     m_isEnabled = true;
     m_messageRouter->enable();
@@ -112,9 +126,12 @@ void AVSConnectionManager::reconnect() {
 
 void AVSConnectionManager::sendMessage(std::shared_ptr<avsCommon::avs::MessageRequest> request) {
     // TODO: ACSDK-421: Implement synchronized state check at a lower level.
+    std::unique_lock<std::mutex> lock{m_synchronizationMutex};
     if (m_isSynchronized) {
+        lock.unlock();
         m_messageRouter->sendMessage(request);
     } else {
+        lock.unlock();
         ACSDK_DEBUG(LX("sendMessageNotSuccessful").d("reason", "notSynchronized"));
         request->onSendCompleted(avsCommon::avs::MessageRequest::Status::NOT_SYNCHRONIZED);
     }
@@ -180,8 +197,14 @@ void AVSConnectionManager::removeMessageObserver(
 }
 
 void AVSConnectionManager::onConnectionStatusChanged(
-  const ConnectionStatusObserverInterface::Status status,
+        const ConnectionStatusObserverInterface::Status status,
         const ConnectionStatusObserverInterface::ChangedReason reason) {
+    if (status == ConnectionStatusObserverInterface::Status::DISCONNECTED || 
+                status == ConnectionStatusObserverInterface::Status::PENDING ) {
+        std::lock_guard<std::mutex>{m_synchronizationMutex};
+        m_isSynchronized = false;
+    }
+
     std::unique_lock<std::mutex> lock{m_connectionStatusObserverMutex};
     std::unordered_set<std::shared_ptr<avsCommon::sdkInterfaces::ConnectionStatusObserverInterface>> observers{m_connectionStatusObservers};
     lock.unlock();
@@ -192,7 +215,14 @@ void AVSConnectionManager::onConnectionStatusChanged(
 }
 
 void AVSConnectionManager::onStateChanged(StateSynchronizerObserverInterface::State newState) {
+    std::unique_lock<std::mutex> lock{m_synchronizationMutex};
     m_isSynchronized = (StateSynchronizerObserverInterface::State::SYNCHRONIZED == newState);
+    if (m_isSynchronized) {
+        lock.unlock();
+        onConnectionStatusChanged(
+                ConnectionStatusObserverInterface::Status::POST_CONNECTED, 
+                ConnectionStatusObserverInterface::ChangedReason::ACL_CLIENT_REQUEST);
+    }
 }
 
 void AVSConnectionManager::receive(const std::string & contextId, const std::string & message) {

ACL/src/CMakeLists.txt

@@ -10,3 +10,6 @@ target_include_directories(ACL PUBLIC ${CURL_INCLUDE_DIRS})
 target_include_directories(ACL PUBLIC "${ACL_SOURCE_DIR}/include")
 target_include_directories(ACL PUBLIC "${AVSCommon_INCLUDE_DIRS}")
 target_link_libraries(ACL ${CURL_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} AVSCommon)
+
+# install target
+asdk_install()

ADSL/include/ADSL/DirectiveRouter.h

@@ -24,19 +24,18 @@
 #include <AVSCommon/AVS/NamespaceAndName.h>
 #include <AVSCommon/AVS/DirectiveHandlerConfiguration.h>
 #include <AVSCommon/AVS/HandlerAndPolicy.h>
+#include <AVSCommon/Utils/RequiresShutdown.h>
 
 namespace alexaClientSDK {
 namespace adsl {
 /**
  * Class to maintain a mapping from @c NamespaceAndName to @c HandlerAndPolicy, and to invoke
  * @c DirectiveHandlerInterface methods on the @c DirectiveHandler registered for a given @c AVSDirective.
  */
-class DirectiveRouter {
+class DirectiveRouter : public avsCommon::utils::RequiresShutdown{
 public:
-    /**
-     * Destructor.
-     */
-    ~DirectiveRouter();
+    /// Constructor.
+    DirectiveRouter();
 
     /**
      * Add mappings from from handler's @c NamespaceAndName values to @c BlockingPolicy values, gotten through the
@@ -100,6 +99,8 @@ class DirectiveRouter {
     bool cancelDirective(std::shared_ptr<avsCommon::avs::AVSDirective> directive);
 
 private:
+    void doShutdown() override;
+
     /**
      * The lifecycle of instances of this class are used to set-up and tear-down around a call to a
      * @c DirectiveHandlerInterface method.  In particular, while instantiated it increments the reference count of
@@ -169,6 +170,21 @@ class DirectiveRouter {
             std::unique_lock<std::mutex>& lock,
             std::shared_ptr<avsCommon::sdkInterfaces::DirectiveHandlerInterface> handler);
 
+    /**
+     * Remove the specified mappings from @c NamespaceAndName values to @c BlockingPolicy values, gotten through the
+     * handler's getConfiguration() method. If any of the specified mappings do not match an existing mapping, the 
+     * entire operation is refused.  This function should be called while holding m_mutex.
+     *
+     * @param handler The handler to remove.
+     * @param[out] releasedHandlers A vector of handlers which need to have onDeregistered() called after releasing the
+     *     lock.
+     * @return @c true if @c handler was removed successfully, else @c false.
+     *     vector indicates an error occurred.
+     */
+    bool removeDirectiveHandlerLocked(
+            std::shared_ptr<avsCommon::sdkInterfaces::DirectiveHandlerInterface> handler,
+            std::vector<std::shared_ptr<avsCommon::sdkInterfaces::DirectiveHandlerInterface>> * releasedHandlers);
+
     /// A mutex used to serialize access to @c m_configuration and @c m_handlerReferenceCounts.
     std::mutex m_mutex;
 

ADSL/include/ADSL/DirectiveSequencer.h

@@ -48,8 +48,6 @@ class DirectiveSequencer : public avsCommon::sdkInterfaces::DirectiveSequencerIn
     static std::unique_ptr<DirectiveSequencerInterface> create(
             std::shared_ptr<avsCommon::sdkInterfaces::ExceptionEncounteredSenderInterface> exceptionSender);
 
-    ~DirectiveSequencer() override;
-
     bool addDirectiveHandler(std::shared_ptr<avsCommon::sdkInterfaces::DirectiveHandlerInterface> handler) override;
 
     bool removeDirectiveHandler(std::shared_ptr<avsCommon::sdkInterfaces::DirectiveHandlerInterface> handler) override;
@@ -58,8 +56,6 @@ class DirectiveSequencer : public avsCommon::sdkInterfaces::DirectiveSequencerIn
 
     bool onDirective(std::shared_ptr<avsCommon::avs::AVSDirective> directive) override;
 
-    void shutdown() override;
-
 private:
     /**
      * Constructor.
@@ -69,6 +65,13 @@ class DirectiveSequencer : public avsCommon::sdkInterfaces::DirectiveSequencerIn
      */
     DirectiveSequencer(std::shared_ptr<avsCommon::sdkInterfaces::ExceptionEncounteredSenderInterface> exceptionSender);
 
+    /**
+     * @copydoc
+     *
+     * This method blocks until all processing of directives has stopped.
+     */
+    void doShutdown() override;
+
     /**
      * Thread method for m_receivingThread.
      */

ADSL/src/CMakeLists.txt

@@ -13,3 +13,6 @@ target_include_directories(ADSL PUBLIC
 target_link_libraries(ADSL
     ${CMAKE_THREAD_LIBS_INIT}
     AVSCommon)
+
+# install target
+asdk_install()

ADSL/src/DirectiveRouter.cpp

@@ -39,18 +39,19 @@ namespace adsl {
 
 using namespace avsCommon::avs;
 using namespace avsCommon::sdkInterfaces;
+using namespace avsCommon::utils;
 
-DirectiveRouter::~DirectiveRouter() {
-    std::lock_guard<std::mutex> lock(m_mutex);
-
-    for (auto item : m_handlerReferenceCounts) {
-        item.first->onDeregistered();
-    }
+DirectiveRouter::DirectiveRouter() : RequiresShutdown{"DirectiveRouter"} {
 }
 
 bool DirectiveRouter::addDirectiveHandler(std::shared_ptr<DirectiveHandlerInterface> handler) {
     std::lock_guard<std::mutex> lock(m_mutex);
 
+    if (isShutdown()) {
+        ACSDK_ERROR(LX("addDirectiveHandlersFailed").d("reason", "isShutdown"));
+        return false;
+    }
+
     if (!handler) {
         ACSDK_ERROR(LX("addDirectiveHandlersFailed").d("reason", "emptyHandler"));
         return false;
@@ -87,11 +88,15 @@ bool DirectiveRouter::addDirectiveHandler(std::shared_ptr<DirectiveHandlerInterf
 
 }
 
-bool DirectiveRouter::removeDirectiveHandler(std::shared_ptr<DirectiveHandlerInterface> handler) {
-    std::unique_lock<std::mutex> lock(m_mutex);
-
+bool DirectiveRouter::removeDirectiveHandlerLocked(
+        std::shared_ptr<DirectiveHandlerInterface> handler,
+        std::vector<std::shared_ptr<DirectiveHandlerInterface>> * releasedHandlers) {
+    if (!releasedHandlers) {
+        ACSDK_ERROR(LX("removeDirectiveHandlersFailed").d("reason", "nullptrReleasedHandlers"));
+        return false;
+    }
     if (!handler) {
-        ACSDK_ERROR(LX("removeDirectiveHandlersFailed").d("reason", "emptyHandler"));
+        ACSDK_ERROR(LX("removeDirectiveHandlersFailed").d("reason", "nullptrHandler"));
         return false;
     }
 
@@ -114,7 +119,6 @@ bool DirectiveRouter::removeDirectiveHandler(std::shared_ptr<DirectiveHandlerInt
      * would create a race condition because that function temporarily releases @c m_mutex when a count goes to zero.
      * Instead, the operation is expanded here with the lock released once we know which handlers to notify.
      */
-    std::vector<std::shared_ptr<DirectiveHandlerInterface>> releasedHandlers;
     for (auto item : configuration) {
         m_configuration.erase(item.first);
         ACSDK_INFO(LX("removeDirectiveHandlers")
@@ -125,10 +129,19 @@ bool DirectiveRouter::removeDirectiveHandler(std::shared_ptr<DirectiveHandlerInt
                 .d("policy", item.second));
         auto it = m_handlerReferenceCounts.find(handler);
         if (0 == --(it->second)) {
-            releasedHandlers.push_back(handler);
+            releasedHandlers->push_back(handler);
             m_handlerReferenceCounts.erase(it);
         }
     }
+    return true;
+}
+
+bool DirectiveRouter::removeDirectiveHandler(std::shared_ptr<DirectiveHandlerInterface> handler) {
+    std::unique_lock<std::mutex> lock(m_mutex);
+    std::vector<std::shared_ptr<DirectiveHandlerInterface>> releasedHandlers;
+    if (!removeDirectiveHandlerLocked(handler, &releasedHandlers)) {
+        return false;
+    }
     lock.unlock();
     for (auto releasedHandler : releasedHandlers) {
         ACSDK_INFO(LX("onDeregisteredCalled").d("handler", releasedHandler.get()));
@@ -210,6 +223,30 @@ bool DirectiveRouter::cancelDirective(std::shared_ptr<avsCommon::avs::AVSDirecti
     return true;
 }
 
+void DirectiveRouter::doShutdown() {
+    std::vector<std::shared_ptr<avsCommon::sdkInterfaces::DirectiveHandlerInterface>> releasedHandlers;
+    std::unique_lock<std::mutex> lock(m_mutex);
+
+    // Should remove all configurations cleanly.
+    size_t numConfigurations = m_configuration.size();
+    for (size_t i = 0; i < numConfigurations && !m_configuration.empty(); ++i) {
+        std::vector<std::shared_ptr<DirectiveHandlerInterface>> handlers;
+        if (removeDirectiveHandlerLocked(m_configuration.begin()->second.handler, &handlers)) {
+            releasedHandlers.insert(releasedHandlers.end(), handlers.begin(), handlers.end());
+        }
+    }
+
+    // For good measure
+    m_configuration.clear();
+
+    lock.unlock();
+
+    for (auto releasedHandler : releasedHandlers) {
+        ACSDK_INFO(LX("onDeregisteredCalled").d("handler", releasedHandler.get()));
+        releasedHandler->onDeregistered();
+    }
+}
+
 DirectiveRouter::HandlerCallScope::HandlerCallScope(
         std::unique_lock<std::mutex>& lock,
         DirectiveRouter* router,

ADSL/src/DirectiveSequencer.cpp

@@ -51,23 +51,6 @@ std::unique_ptr<DirectiveSequencerInterface> DirectiveSequencer::create(
     return std::unique_ptr<DirectiveSequencerInterface>(new DirectiveSequencer(exceptionSender));
 }
 
-DirectiveSequencer::~DirectiveSequencer() {
-    shutdown();
-}
-
-void DirectiveSequencer::shutdown() {
-    ACSDK_INFO(LX("shutdown"));
-    {
-        std::lock_guard<std::mutex> lock(m_mutex);
-        m_isShuttingDown = true;
-        m_wakeReceivingLoop.notify_one();
-    }
-    if (m_receivingThread.joinable()) {
-        m_receivingThread.join();
-    }
-    m_directiveProcessor->shutdown();
-}
-
 bool DirectiveSequencer::addDirectiveHandler(std::shared_ptr<DirectiveHandlerInterface> handler) {
     return m_directiveRouter.addDirectiveHandler(handler);
 }
@@ -101,13 +84,29 @@ bool DirectiveSequencer::onDirective(std::shared_ptr<AVSDirective> directive) {
 
 DirectiveSequencer::DirectiveSequencer(
         std::shared_ptr<avsCommon::sdkInterfaces::ExceptionEncounteredSenderInterface> exceptionSender) :
+        DirectiveSequencerInterface{"DirectiveSequencer"},
         m_mutex{},
         m_exceptionSender{exceptionSender},
         m_isShuttingDown{false} {
     m_directiveProcessor = std::make_shared<DirectiveProcessor>(&m_directiveRouter);
     m_receivingThread = std::thread(&DirectiveSequencer::receivingLoop, this);
 }
 
+void DirectiveSequencer::doShutdown() {
+    ACSDK_INFO(LX("doShutdown"));
+    {
+        std::lock_guard<std::mutex> lock(m_mutex);
+        m_isShuttingDown = true;
+        m_wakeReceivingLoop.notify_one();
+    }
+    if (m_receivingThread.joinable()) {
+        m_receivingThread.join();
+    }
+    m_directiveProcessor->shutdown();
+    m_directiveRouter.shutdown();
+    m_exceptionSender.reset();
+}
+
 void DirectiveSequencer::receivingLoop() {
     auto wake = [this]() {
         return !m_receivingQueue.empty() || m_isShuttingDown;

ADSL/test/ADSL/MockDirectiveSequencer.h

@@ -34,7 +34,9 @@ namespace test {
  */
 class MockDirectiveSequencer : public avsCommon::sdkInterfaces::DirectiveSequencerInterface {
 public:
-    MOCK_METHOD0(shutdown,void());
+    MockDirectiveSequencer();
+
+    MOCK_METHOD0(doShutdown,void());
 
     MOCK_METHOD1(addDirectiveHandler, 
             bool(std::shared_ptr<avsCommon::sdkInterfaces::DirectiveHandlerInterface> handler));
@@ -47,6 +49,10 @@ class MockDirectiveSequencer : public avsCommon::sdkInterfaces::DirectiveSequenc
     MOCK_METHOD1(onDirective, bool(std::shared_ptr<avsCommon::avs::AVSDirective> directive));
 };
 
+inline MockDirectiveSequencer::MockDirectiveSequencer() :
+        avsCommon::sdkInterfaces::DirectiveSequencerInterface{"MockDirectiveSequencer"} {
+}
+
 } // namespace test
 } // namespace adsl
 } // namespace alexaClientSDK