Non-trivial fix for a keyboard control shutdown deadlock.

Author: Mysticial

Common/Cpp/CancellableScope.cpp

@@ -10,6 +10,7 @@
 #include <mutex>
 #include <condition_variable>
 #include "Exceptions.h"
+#include "ListenerSet.h"
 #include "Containers/Pimpl.tpp"
 #include "Concurrency/SpinLock.h"
 #include "CancellableScope.h"
@@ -28,9 +29,17 @@ struct CancellableData{
     std::atomic<bool> cancelled;
     mutable SpinLock lock;
     std::exception_ptr exception;
+    ListenerSet<Cancellable::CancelListener> m_listeners;
 };
 
 
+void Cancellable::add_cancel_listener(CancelListener& listener){
+    m_impl->m_listeners.add(listener);
+}
+void Cancellable::remove_cancel_listener(CancelListener& listener){
+    m_impl->m_listeners.remove(listener);
+}
+
 
 Cancellable::Cancellable()
     : m_impl(CONSTRUCT_TOKEN)
@@ -56,10 +65,15 @@ bool Cancellable::cancel(std::exception_ptr exception) noexcept{
     if (exception && !data.exception){
         data.exception = std::move(exception);
     }
-    if (data.cancelled.load(std::memory_order_acquire)){
+    if (data.cancelled.load(std::memory_order_relaxed)){
         return true;
     }
-    return data.cancelled.exchange(true, std::memory_order_relaxed);
+    data.cancelled.store(true, std::memory_order_release);
+//    if (data.cancelled.exchange(true, std::memory_order_relaxed)){
+//        return true;
+//    }
+    m_impl->m_listeners.run_method_with_duplicates(&CancelListener::on_cancellable_cancel);
+    return false;
 }
 void Cancellable::throw_if_cancelled() const{
     auto scope_check = m_sanitizer.check_scope();

Common/Cpp/CancellableScope.h

@@ -45,6 +45,14 @@ struct CancellableData;
 class Cancellable{
     Cancellable(const Cancellable&) = delete;
     void operator=(const Cancellable&) = delete;
+public:
+    struct CancelListener{
+        virtual void on_cancellable_cancel() = 0;
+    };
+    void add_cancel_listener(CancelListener& listener);
+    void remove_cancel_listener(CancelListener& listener);
+
+
 public:
     virtual ~Cancellable();
 

Common/Cpp/ListenerSet.h

@@ -202,7 +202,7 @@ void ListenerSet<ListenerType>::run_method_with_duplicates(Function function, Ar
         ListenerType& listener = *item.first;
         size_t count = item.second;
         do{
-            (listener->*function)(std::forward<Args>(args)...);
+            (listener.*function)(std::forward<Args>(args)...);
         }while (--count);
     }
 }

SerialPrograms/Source/CommonFramework/Globals.cpp

@@ -26,7 +26,7 @@ namespace PokemonAutomation{
 const bool IS_BETA_VERSION = true;
 const int PROGRAM_VERSION_MAJOR = 0;
 const int PROGRAM_VERSION_MINOR = 60;
-const int PROGRAM_VERSION_PATCH = 5;
+const int PROGRAM_VERSION_PATCH = 6;
 
 const std::string PROGRAM_VERSION_BASE =
     "v" + std::to_string(PROGRAM_VERSION_MAJOR) +

SerialPrograms/Source/Controllers/Controller.h

@@ -180,16 +180,16 @@ class AbstractController{
     //  empty out rather than entering itself into the queue.
     //  If a cancellation happens inside this function, it will immediately
     //  throw an OperationCancelledException.
-    virtual void wait_for_all(const Cancellable* cancellable) = 0;
+    virtual void wait_for_all(Cancellable* cancellable) = 0;
 
     //  Tell the scheduler to wait for all pending commands to finish
     //  (including cooldowns) before executing further instructions.
     //  This is used to prevent hanging commands from overlapping with new
     //  commands issued after this barrier.
-    virtual void issue_barrier(const Cancellable* cancellable) = 0;
+    virtual void issue_barrier(Cancellable* cancellable) = 0;
 
     //  Do nothing for this much time.
-    virtual void issue_nop(const Cancellable* cancellable, Milliseconds duration) = 0;
+    virtual void issue_nop(Cancellable* cancellable, Milliseconds duration) = 0;
 
 
 public:
@@ -248,7 +248,7 @@ class ControllerContext final : public CancellableScope{
 
 
 public:
-    void wait_for_all_requests() const{
+    void wait_for_all_requests(){
         auto scope = m_lifetime_sanitizer.check_scope();
         m_controller.wait_for_all(this);
     }

SerialPrograms/Source/Controllers/KeyboardInput/KeyboardInput.cpp

@@ -24,7 +24,6 @@ namespace PokemonAutomation{
 KeyboardInputController::~KeyboardInputController() = default;
 KeyboardInputController::KeyboardInputController(Logger& logger, bool enabled)
     : m_logger(logger)
-    , m_stop(false)
 {}
 
 void KeyboardInputController::start(){
@@ -36,9 +35,7 @@ void KeyboardInputController::start(){
     });
 }
 void KeyboardInputController::stop() noexcept{
-    bool expected = false;
-    if (!m_stop.compare_exchange_strong(expected, true)){
-        //  Already stopped.
+    if (cancel(nullptr)){
         return;
     }
     {
@@ -95,7 +92,7 @@ void KeyboardInputController::thread_loop(){
     bool last_neutral = true;
     WallClock last_press = current_time();
     while (true){
-        if (m_stop.load(std::memory_order_acquire)){
+        if (cancelled()){
             return;
         }
 
@@ -170,7 +167,7 @@ void KeyboardInputController::thread_loop(){
 
         //  Wait for next event.
         std::unique_lock<std::mutex> lg(m_sleep_lock);
-        if (m_stop.load(std::memory_order_acquire)){
+        if (cancelled()){
             return;
         }
 

SerialPrograms/Source/Controllers/KeyboardInput/KeyboardInput.h

@@ -25,7 +25,7 @@ namespace PokemonAutomation{
 
 
 
-class KeyboardInputController{
+class KeyboardInputController : public Cancellable{
 public:
     KeyboardInputController(Logger& logger, bool enabled);
     virtual ~KeyboardInputController();
@@ -59,7 +59,7 @@ class KeyboardInputController{
     SpinLock m_state_lock;
     KeyboardStateTracker m_state_tracker;
 
-    std::atomic<bool> m_stop;
+//    std::atomic<bool> m_stop;
 
     std::mutex m_sleep_lock;
     std::condition_variable m_cv;
@@ -77,14 +77,14 @@ class KeyboardManager : public KeyboardInputController, public KeyboardEventHand
         , m_controller(&controller)
     {}
     void stop() noexcept{
+        KeyboardInputController::stop();
         {
             WriteSpinLock lg(m_lock);
             if (m_controller == nullptr){
                 return;
             }
             m_controller = nullptr;
         }
-        KeyboardInputController::stop();
     }
 
     virtual std::unique_ptr<ControllerState> make_state() const override{

SerialPrograms/Source/Controllers/Schedulers/ControllerWithScheduler.h

@@ -34,7 +34,7 @@ class ControllerWithScheduler{
 public:
     //  Superscalar Commands (the "ssf" framework)
 
-    void issue_barrier(const Cancellable* cancellable){
+    void issue_barrier(Cancellable* cancellable){
         SuperscalarScheduler::Schedule schedule;
         std::lock_guard<std::mutex> lg0(m_issue_lock);
         {
@@ -46,7 +46,7 @@ class ControllerWithScheduler{
             m_logger.log("issue_barrier()", COLOR_DARKGREEN);
         }
     }
-    void issue_nop(const Cancellable* cancellable, Milliseconds duration){
+    void issue_nop(Cancellable* cancellable, Milliseconds duration){
         SuperscalarScheduler::Schedule schedule;
         std::lock_guard<std::mutex> lg0(m_issue_lock);
         {
@@ -68,11 +68,11 @@ class ControllerWithScheduler{
 
 protected:
     virtual void execute_state(
-        const Cancellable* cancellable,
+        Cancellable* cancellable,
         const SuperscalarScheduler::ScheduleEntry& entry
     ) = 0;
     virtual void execute_schedule(
-        const Cancellable* cancellable,
+        Cancellable* cancellable,
         const SuperscalarScheduler::Schedule& schedule
     ){
         for (const SuperscalarScheduler::ScheduleEntry& entry : schedule){

SerialPrograms/Source/Controllers/SerialPABotBase/Connection/BotBase.h

@@ -19,7 +19,7 @@ class BotBaseControllerContext;
 
 
 
-class BotBaseController{
+class BotBaseController : public Cancellable::CancelListener{
 public:
     using ContextType = BotBaseControllerContext;
 
@@ -38,10 +38,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;
+    virtual void wait_for_all_requests(Cancellable* cancelled = nullptr) = 0;
 
     //  Stop all pending commands. This wipes the command queue on both sides
     //  and stops any currently executing command.
@@ -59,15 +57,15 @@ class BotBaseController{
 public:
     virtual bool try_issue_request(
         const BotBaseRequest& request,
-        const Cancellable* cancelled = nullptr
+        Cancellable* cancelled = nullptr
     ) = 0;
     virtual void issue_request(
         const BotBaseRequest& request,
-        const Cancellable* cancelled = nullptr
+        Cancellable* cancelled = nullptr
     ) = 0;
     virtual BotBaseMessage issue_request_and_wait(
         const BotBaseRequest& request,
-        const Cancellable* cancelled = nullptr
+        Cancellable* cancelled = nullptr
     ) = 0;
 
 };