From 01c5dcb6b731cec3407e89694158ea43799d1808 Mon Sep 17 00:00:00 2001 From: Michael Carlberg Date: Tue, 18 Oct 2016 17:55:34 +0200 Subject: [PATCH] fix: Guarded module teardown --- CMakeLists.txt | 2 +- include/components/controller.hpp | 9 ----- include/modules/bspwm.hpp | 11 +++--- include/modules/meta.hpp | 59 ++++++++++++++++--------------- include/modules/mpd.hpp | 36 ++++++++++--------- include/modules/network.hpp | 5 +++ include/modules/volume.hpp | 4 +++ 7 files changed, 67 insertions(+), 59 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 45630002..a67fb858 100644 --- a/CMakeLists.txt +++ b/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) diff --git a/include/components/controller.hpp b/include/components/controller.hpp index 682e2264..3fb4da5a 100644 --- a/include/components/controller.hpp +++ b/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) { diff --git a/include/modules/bspwm.hpp b/include/modules/bspwm.hpp index 4af8a0c3..b63b2915 100644 --- a/include/modules/bspwm.hpp +++ b/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()); diff --git a/include/modules/meta.hpp b/include/modules/meta.hpp index 3ef00809..7eb66e15 100644 --- a/include/modules/meta.hpp +++ b/include/modules/meta.hpp @@ -178,9 +178,9 @@ namespace modules { , m_formatter(make_unique(m_conf, m_name)) {} ~module() { - if (enabled()) - stop(); + stop(); + this->update_lock.unlock(); std::lock_guard lck(this->update_lock); { if (m_broadcast_thread.joinable()) @@ -210,17 +210,19 @@ namespace modules { } void stop() { - wakeup(); + if (!enabled()) + return; + std::unique_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 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 lck(m_sleeplock); m_log.trace("%s: Release sleep lock", name()); + // std::unique_lock 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 lck(this->update_lock); - if (CAST_MODULE(Impl)->update()) - CAST_MODULE(Impl)->broadcast(); + { + std::lock_guard 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 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 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 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 lck(this->update_lock); if (w->poll(1000 / watches.size())) { auto event = w->get_event(); diff --git a/include/modules/mpd.hpp b/include/modules/mpd.hpp index 532126c8..acd8ce56 100644 --- a/include/modules/mpd.hpp +++ b/include/modules/mpd.hpp @@ -16,21 +16,6 @@ namespace modules { public: using event_module::event_module; - ~mpd_module() { - std::lock_guard 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(name(), "host", m_host); m_port = m_conf.get(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; } } diff --git a/include/modules/network.hpp b/include/modules/network.hpp index 0d4fe0b8..c0d79fa2 100644 --- a/include/modules/network.hpp +++ b/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(m_wireless.get()) : dynamic_cast(m_wired.get()); diff --git a/include/modules/volume.hpp b/include/modules/volume.hpp index ff978310..5254a4ab 100644 --- a/include/modules/volume.hpp +++ b/include/modules/volume.hpp @@ -108,6 +108,10 @@ namespace modules { // }}} } + void teardown() { + m_mixers.clear(); + } + bool has_event() { // Poll for mixer and control events {{{