fix: Guarded module teardown

CMakeLists.txt

@@ -8,7 +8,7 @@ set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${PROJECT_SOURCE_DIR}/cmake/modules)
 
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++14 -Wall -Wno-unused-parameter -Wno-unused-local-typedefs")
 set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DDEBUG -O0 -g2")
-set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O2")
+set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3")
 
 set(CMAKE_C_STANDARD 11)
 set(CMAKE_C_STANDARD_REQUIRED ON)

include/components/controller.hpp

@@ -102,15 +102,6 @@ class controller {
       }
     }
 
-    m_log.trace("controller: Stop modules");
-    for (auto&& block : m_modules) {
-      for (auto&& module : block.second) {
-        module->on_update.disconnect(this, &controller::on_module_update);
-        module->on_stop.disconnect(this, &controller::on_module_stop);
-        module->stop();
-      }
-    }
-
     if (!m_threads.empty()) {
       m_log.trace("controller: Join active threads");
       for (auto&& thread : m_threads) {

include/modules/bspwm.hpp

@@ -43,11 +43,6 @@ namespace modules {
    public:
     using event_module::event_module;
 
-    ~bspwm_module() {
-      if (m_subscriber)
-        m_subscriber->disconnect();
-    }
-
     void setup() {
       m_monitor = m_bar.monitor->name;
       m_log.trace("%s: Primary monitor '%s'", name(), m_monitor);
@@ -105,6 +100,12 @@ namespace modules {
       // }}}
     }
 
+    void stop() {
+      if (m_subscriber)
+        m_subscriber->disconnect();
+      event_module::stop();
+    }
+
     bool has_event() {
       if (m_subscriber->poll(POLLHUP, 0)) {
         m_log.warn("%s: Reconnecting to socket...", name());

include/modules/meta.hpp

@@ -178,9 +178,9 @@ namespace modules {
         , m_formatter(make_unique<module_formatter>(m_conf, m_name)) {}
 
     ~module() {
-      if (enabled())
-        stop();
+      stop();
 
+      this->update_lock.unlock();
       std::lock_guard<threading_util::spin_lock> lck(this->update_lock);
       {
         if (m_broadcast_thread.joinable())
@@ -210,17 +210,19 @@ namespace modules {
     }
 
     void stop() {
-      wakeup();
+      if (!enabled())
+        return;
+      std::unique_lock<threading_util::spin_lock> lck(this->update_lock);
+      enable(false);
+      CAST_MODULE(Impl)->teardown();
+      lck.unlock();
+      m_log.trace("%s: Stop", name());
+      if (!on_stop.empty())
+        on_stop.emit(name());
+    }
 
-      std::lock_guard<threading_util::spin_lock> lck(this->update_lock);
-      {
-        if (!enabled())
-          return;
-        m_log.trace("%s: Stop", name());
-        enable(false);
-        if (!on_stop.empty())
-          on_stop.emit(name());
-      }
+    void teardown() {
+      wakeup();
     }
 
     void refresh() {
@@ -270,8 +272,8 @@ namespace modules {
     }
 
     void wakeup() {
-      std::unique_lock<std::mutex> lck(m_sleeplock);
       m_log.trace("%s: Release sleep lock", name());
+      // std::unique_lock<std::mutex> lck(m_sleeplock);
       m_sleephandler.notify_all();
     }
 
@@ -387,9 +389,11 @@ namespace modules {
         CAST_MODULE(Impl)->setup();
 
         while (CONST_CAST_MODULE(Impl).enabled()) {
-          std::lock_guard<threading_util::spin_lock> lck(this->update_lock);
-          if (CAST_MODULE(Impl)->update())
-            CAST_MODULE(Impl)->broadcast();
+          {
+            std::lock_guard<threading_util::spin_lock> lck(this->update_lock);
+            if (CAST_MODULE(Impl)->update())
+              CAST_MODULE(Impl)->broadcast();
+          }
           CAST_MODULE(Impl)->sleep(m_interval);
         }
       } catch (const std::exception& err) {
@@ -423,18 +427,17 @@ namespace modules {
         CAST_MODULE(Impl)->broadcast();
 
         while (CONST_CAST_MODULE(Impl).enabled()) {
-          std::unique_lock<threading_util::spin_lock> lck(this->update_lock);
-
-          if (!CAST_MODULE(Impl)->has_event())
-            continue;
-
-          if (!CAST_MODULE(Impl)->update())
-            continue;
-
-          CAST_MODULE(Impl)->broadcast();
-
-          lck.unlock();
           CAST_MODULE(Impl)->idle();
+          {
+            std::lock_guard<threading_util::spin_lock> lck(this->update_lock);
+
+            if (!CAST_MODULE(Impl)->has_event())
+              continue;
+            if (!CAST_MODULE(Impl)->update())
+              continue;
+
+            CAST_MODULE(Impl)->broadcast();
+          }
         }
       } catch (const std::exception& err) {
         this->m_log.err("%s: %s", this->name(), err.what());
@@ -465,7 +468,6 @@ namespace modules {
         CAST_MODULE(Impl)->broadcast();
 
         while (CAST_MODULE(Impl)->enabled()) {
-          std::lock_guard<threading_util::spin_lock> lck(this->update_lock);
           CAST_MODULE(Impl)->poll_events();
         }
       } catch (const std::exception& err) {
@@ -503,6 +505,7 @@ namespace modules {
       while (CONST_CAST_MODULE(Impl).enabled()) {
         for (auto&& w : watches) {
           this->m_log.trace("%s: Poll inotify watch %s", this->name(), w->path());
+          std::lock_guard<threading_util::spin_lock> lck(this->update_lock);
 
           if (w->poll(1000 / watches.size())) {
             auto event = w->get_event();

include/modules/mpd.hpp

@@ -16,21 +16,6 @@ namespace modules {
    public:
     using event_module::event_module;
 
-    ~mpd_module() {
-      std::lock_guard<threading_util::spin_lock> lck(this->update_lock);
-      if (m_mpd && m_mpd->connected()) {
-        try {
-          m_mpd->disconnect();
-        } catch (const mpd_exception& e) {
-          m_log.trace("%s: %s", name(), e.what());
-        }
-      }
-    }
-
-    void idle() {
-      sleep(80ms);
-    }
-
     void setup() {
       m_host = m_conf.get<string>(name(), "host", m_host);
       m_port = m_conf.get<unsigned int>(name(), "port", m_port);
@@ -103,6 +88,26 @@ namespace modules {
       }
     }
 
+    void teardown() {
+      if (m_mpd && m_mpd->connected()) {
+        try {
+          m_mpd->disconnect();
+        } catch (const mpd_exception& e) {
+          m_log.trace("%s: %s", name(), e.what());
+        }
+      } else {
+        wakeup();
+      }
+    }
+
+    void idle() {
+      if (m_mpd && m_mpd->connected())
+        sleep(80ms);
+      else {
+        sleep(10s);
+      }
+    }
+
     bool has_event() {
       if (!m_mpd->connected()) {
         m_connection_state_broadcasted = false;
@@ -114,7 +119,6 @@ namespace modules {
         }
 
         if (!m_mpd->connected()) {
-          sleep(2s);
           return false;
         }
       }

include/modules/network.hpp

@@ -74,6 +74,11 @@ namespace modules {
         m_threads.emplace_back(thread(&network_module::subthread_routine, this));
     }
 
+    void teardown() {
+      m_wireless.reset();
+      m_wired.reset();
+    }
+
     bool update() {
       net::network* network = m_wireless ? dynamic_cast<net::network*>(m_wireless.get())
                                          : dynamic_cast<net::network*>(m_wired.get());

include/modules/volume.hpp

@@ -108,6 +108,10 @@ namespace modules {
       // }}}
     }
 
+    void teardown() {
+      m_mixers.clear();
+    }
+
     bool has_event() {
       // Poll for mixer and control events {{{