Fix hang when closing connection while connection is hung.

Author: Mysticial

ClientSource/Connection/BotBase.h

@@ -36,6 +36,8 @@ class BotBaseController{
     virtual State state() const = 0;
     virtual size_t queue_limit() const = 0;
 
+    virtual void notify_all() = 0;
+
     //  Waits for all pending requests to finish.
     virtual void wait_for_all_requests(const Cancellable* cancelled = nullptr) = 0;
 

ClientSource/Connection/PABotBase.cpp

@@ -124,6 +124,11 @@ void PABotBase::stop(){
     //  it is safe to destruct.
     m_state.store(State::STOPPED, std::memory_order_release);
 }
+void PABotBase::notify_all(){
+    std::unique_lock<std::mutex> lg(m_sleep_lock);
+    m_cv.notify_all();
+}
+
 void PABotBase::set_queue_limit(size_t queue_limit){
     m_max_pending_requests.store(queue_limit, std::memory_order_relaxed);
 }
@@ -810,13 +815,17 @@ BotBaseMessage PABotBase::issue_request_and_wait(
     }
 
     uint64_t seqnum = issue_request(cancelled, request, false);
-    return wait_for_request(seqnum);
+    return wait_for_request(seqnum, cancelled);
 }
-BotBaseMessage PABotBase::wait_for_request(uint64_t seqnum){
+BotBaseMessage PABotBase::wait_for_request(uint64_t seqnum, const Cancellable* cancelled){
     auto scope_check = m_sanitizer.check_scope();
 
     std::unique_lock<std::mutex> lg(m_sleep_lock);
     while (true){
+        if (cancelled && cancelled->cancelled()){
+            throw OperationCancelledException();
+        }
+
         {
             WriteSpinLock slg(m_state_lock, "PABotBase::issue_request_and_wait()");
             auto iter = m_pending_requests.find(seqnum);

ClientSource/Connection/PABotBase.h

@@ -68,6 +68,7 @@ class PABotBase : public BotBaseController, private PABotBaseConnection{
     virtual State state() const override{
         return m_state.load(std::memory_order_acquire);
     }
+    virtual void notify_all() override;
 
     virtual size_t queue_limit() const override{
         return m_max_pending_requests.load(std::memory_order_relaxed);
@@ -170,7 +171,7 @@ class PABotBase : public BotBaseController, private PABotBaseConnection{
     ) override;
 
 private:
-    BotBaseMessage wait_for_request(uint64_t seqnum);
+    BotBaseMessage wait_for_request(uint64_t seqnum, const Cancellable* cancelled = nullptr);
 
 private:
     Logger& m_logger;

SerialPrograms/Source/NintendoSwitch/Controllers/SerialPABotBase/NintendoSwitch_PokkenController.cpp

@@ -43,8 +43,12 @@ SerialPABotBase_PokkenController::SerialPABotBase_PokkenController(
 {}
 SerialPABotBase_PokkenController::~SerialPABotBase_PokkenController(){
     stop();
+    m_scope.cancel(nullptr);
     {
         std::unique_lock<std::mutex> lg(m_sleep_lock);
+        if (m_serial){
+            m_serial->notify_all();
+        }
         m_cv.notify_all();
         m_stopping.store(true, std::memory_order_relaxed);
     }
@@ -100,7 +104,6 @@ void SerialPABotBase_PokkenController::push_state(const Cancellable* cancellable
 
 
 void SerialPABotBase_PokkenController::status_thread(){
-
     constexpr std::chrono::milliseconds PERIOD(1000);
     std::atomic<WallClock> last_ack(current_time());
 
@@ -134,8 +137,6 @@ void SerialPABotBase_PokkenController::status_thread(){
         }
     });
 
-    CancellableHolder<CancellableScope> scope;
-
     WallClock next_ping = current_time();
     while (true){
         if (m_stopping.load(std::memory_order_relaxed) || !m_handle.is_ready()){
@@ -147,7 +148,7 @@ void SerialPABotBase_PokkenController::status_thread(){
             pabb_MsgAckRequestI32 response;
             m_serial->issue_request_and_wait(
                 NintendoSwitch::DeviceRequest_system_clock(),
-                &scope
+                &m_scope
             ).convert<PABB_MSG_ACK_REQUEST_I32>(logger(), response);
             last_ack.store(current_time(), std::memory_order_relaxed);
             uint32_t wallclock = response.data;
@@ -162,6 +163,8 @@ void SerialPABotBase_PokkenController::status_thread(){
                     theme_friendly_darkblue()
                 );
             }
+        }catch (OperationCancelledException&){
+            break;
         }catch (InvalidConnectionStateException&){
             break;
         }catch (SerialProtocolException& e){

SerialPrograms/Source/NintendoSwitch/Controllers/SerialPABotBase/NintendoSwitch_PokkenController.h

@@ -36,6 +36,7 @@ class SerialPABotBase_PokkenController final : public SerialPABotBase_ProControl
     void status_thread();
 
 private:
+    CancellableHolder<CancellableScope> m_scope;
     std::atomic<bool> m_stopping;
     std::mutex m_sleep_lock;
     std::condition_variable m_cv;

SerialPrograms/Source/NintendoSwitch/Controllers/SerialPABotBase/NintendoSwitch_WirelessProController.cpp

@@ -10,6 +10,7 @@
 #include "Common/Cpp/Concurrency/ReverseLockGuard.h"
 #include "Common/NintendoSwitch/NintendoSwitch_Protocol_ESP32.h"
 #include "ClientSource/Libraries/MessageConverter.h"
+#include "CommonFramework/GlobalSettingsPanel.h"
 #include "CommonFramework/Options/Environment/ThemeSelectorOption.h"
 #include "Controllers/ControllerCapability.h"
 #include "NintendoSwitch/Commands/NintendoSwitch_Messages_Device.h"
@@ -43,8 +44,12 @@ SerialPABotBase_WirelessProController::SerialPABotBase_WirelessProController(
 {}
 SerialPABotBase_WirelessProController::~SerialPABotBase_WirelessProController(){
     stop();
+    m_scope.cancel(nullptr);
     {
         std::unique_lock<std::mutex> lg(m_sleep_lock);
+        if (m_serial){
+            m_serial->notify_all();
+        }
         m_cv.notify_all();
         m_stopping.store(true, std::memory_order_relaxed);
     }
@@ -82,6 +87,21 @@ class SerialPABotBase_WirelessProController::MessageControllerState : public Bot
 };
 
 int register_message_converters_ESP32(){
+    register_message_converter(
+        PABB_MSG_ESP32_REQUEST_STATUS,
+        [](const std::string& body){
+            //  Disable this by default since it's very spammy.
+            if (!GlobalSettings::instance().LOG_EVERYTHING){
+                return std::string();
+            }
+            std::ostringstream ss;
+            ss << "ESP32_controller_status() - ";
+            if (body.size() != sizeof(pabb_esp32_RequestStatus)){ ss << "(invalid size)" << std::endl; return ss.str(); }
+            const auto* params = (const pabb_esp32_RequestStatus*)body.c_str();
+            ss << "seqnum = " << (uint64_t)params->seqnum;
+            return ss.str();
+        }
+    );
     register_message_converter(
         PABB_MSG_ESP32_REPORT,
         [](const std::string& body){
@@ -181,7 +201,6 @@ void SerialPABotBase_WirelessProController::push_state(const Cancellable* cancel
 
 
 void SerialPABotBase_WirelessProController::status_thread(){
-
     constexpr std::chrono::milliseconds PERIOD(1000);
     std::atomic<WallClock> last_ack(current_time());
 
@@ -215,8 +234,6 @@ void SerialPABotBase_WirelessProController::status_thread(){
         }
     });
 
-    CancellableHolder<CancellableScope> scope;
-
     WallClock next_ping = current_time();
     while (true){
         if (m_stopping.load(std::memory_order_relaxed) || !m_handle.is_ready()){
@@ -228,7 +245,7 @@ void SerialPABotBase_WirelessProController::status_thread(){
             pabb_MsgAckRequestI32 response;
             m_serial->issue_request_and_wait(
                 MessageControllerStatus(),
-                &scope
+                &m_scope
             ).convert<PABB_MSG_ACK_REQUEST_I32>(logger(), response);
             last_ack.store(current_time(), std::memory_order_relaxed);
 

SerialPrograms/Source/NintendoSwitch/Controllers/SerialPABotBase/NintendoSwitch_WirelessProController.h

@@ -39,6 +39,7 @@ class SerialPABotBase_WirelessProController final : public SerialPABotBase_ProCo
     void status_thread();
 
 private:
+    CancellableHolder<CancellableScope> m_scope;
     std::atomic<bool> m_stopping;
     std::mutex m_sleep_lock;
     std::condition_variable m_cv;

SerialPrograms/Source/Tests/TestUtils.h

@@ -62,6 +62,8 @@ class DummyBotBase : public BotBaseController{
     virtual State state() const override { return State::RUNNING; }
     virtual size_t queue_limit() const override { return PABB_DEVICE_QUEUE_SIZE; }
 
+    virtual void notify_all() override{}
+
     virtual void wait_for_all_requests(const Cancellable* cancelled = nullptr) override {}
 
     virtual bool try_stop_all_commands() override { return true; }