commit 7aa6bfaffb56158165931abab45fb542644b1fe8
parent bb2669c3dd3c79c18f5ebb9bf307abc176d7bc73
Author: cfillion <cfillion@users.noreply.github.com>
Date: Sat, 18 Aug 2018 21:38:09 -0400
rewrite main thread notification code
Diffstat:
| A | src/event.cpp | | | 81 | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ |
| A | src/event.hpp | | | 59 | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ |
| M | src/thread.cpp | | | 69 | +++++++-------------------------------------------------------------- |
| M | src/thread.hpp | | | 31 | +++---------------------------- |
| A | test/event.cpp | | | 66 | ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ |
5 files changed, 216 insertions(+), 90 deletions(-)
diff --git a/src/event.cpp b/src/event.cpp
@@ -0,0 +1,81 @@
+/* ReaPack: Package manager for REAPER
+ * Copyright (C) 2015-2018 Christian Fillion
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "event.hpp"
+
+#include <reaper_plugin_functions.h>
+
+static std::weak_ptr<EventLoop> s_loop;
+
+EventLoop::EventLoop()
+{
+ plugin_register("timer", reinterpret_cast<void *>(&mainThreadTimer));
+}
+
+EventLoop::~EventLoop()
+{
+ plugin_register("-timer", reinterpret_cast<void *>(&mainThreadTimer));
+}
+
+void EventLoop::mainThreadTimer()
+{
+ s_loop.lock()->processQueue();
+}
+
+void EventLoop::push(EventEmitter *emitter, const Event event)
+{
+ std::lock_guard<std::mutex> guard(m_mutex);
+ m_queue.insert({emitter, event});
+}
+
+void EventLoop::forget(EventEmitter *emitter)
+{
+ std::lock_guard<std::mutex> guard(m_mutex);
+ m_queue.erase(emitter);
+}
+
+void EventLoop::processQueue()
+{
+ decltype(m_queue) events;
+
+ {
+ std::lock_guard<std::mutex> guard(m_mutex);
+ std::swap(events, m_queue);
+ }
+
+ for(const auto &[emitter, event] : events)
+ emitter->eventHandler(event);
+}
+
+EventEmitter::EventEmitter()
+{
+ if(s_loop.expired())
+ s_loop = m_loop = std::make_shared<EventLoop>();
+ else
+ m_loop = s_loop.lock();
+}
+
+EventEmitter::~EventEmitter()
+{
+ if(s_loop.use_count() > 1)
+ m_loop->forget(this);
+}
+
+void EventEmitter::emit(const Event event)
+{
+ m_loop->push(this, event);
+}
diff --git a/src/event.hpp b/src/event.hpp
@@ -0,0 +1,59 @@
+/* ReaPack: Package manager for REAPER
+ * Copyright (C) 2015-2018 Christian Fillion
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef REAPACK_EVENT_HPP
+#define REAPACK_EVENT_HPP
+
+#include <map>
+#include <memory>
+#include <mutex>
+
+typedef intptr_t Event;
+class EventEmitter;
+
+class EventLoop {
+public:
+ EventLoop();
+ ~EventLoop();
+
+ void push(EventEmitter *, Event);
+ void forget(EventEmitter *);
+
+private:
+ static void mainThreadTimer();
+ void processQueue();
+
+ std::mutex m_mutex;
+ std::multimap<EventEmitter *, Event> m_queue;
+};
+
+class EventEmitter {
+public:
+ EventEmitter();
+ virtual ~EventEmitter();
+
+ void emit(Event);
+
+protected:
+ friend EventLoop;
+ virtual void eventHandler(Event) = 0;
+
+private:
+ std::shared_ptr<EventLoop> m_loop;
+};
+
+#endif
diff --git a/src/thread.cpp b/src/thread.cpp
@@ -26,21 +26,18 @@
using namespace std;
-ThreadNotifier *ThreadNotifier::s_instance = nullptr;
-
-ThreadTask::ThreadTask()
- : m_state(Idle), m_abort(false)
+ThreadTask::ThreadTask() : m_state(Idle), m_abort(false)
{
- ThreadNotifier::get()->start();
}
ThreadTask::~ThreadTask()
{
- ThreadNotifier::get()->stop();
}
-void ThreadTask::setState(const State state)
+void ThreadTask::eventHandler(const Event event)
{
+ const State state = static_cast<State>(event);
+
// The task may have been aborted while the task was running or just before
// the finish notification got received in the main thread.
m_state = aborted() ? Aborted : state;
@@ -74,12 +71,12 @@ void ThreadTask::exec()
State state = Aborted;
if(!aborted()) {
- ThreadNotifier::get()->notify({this, Running});
+ emit(Running);
state = run() ? Success : Failure;
}
- ThreadNotifier::get()->notify({this, state});
-};
+ emit(state);
+}
WorkerThread::WorkerThread() : m_stop(false), m_thread(&WorkerThread::run, this)
{
@@ -142,7 +139,6 @@ void WorkerThread::push(ThreadTask *task)
{
lock_guard<mutex> guard(m_mutex);
- task->setState(ThreadTask::Queued);
m_queue.push(task);
m_wake.notify_one();
}
@@ -186,54 +182,3 @@ void ThreadPool::abort()
m_onAbort();
}
-
-ThreadNotifier *ThreadNotifier::get()
-{
- if(!s_instance)
- s_instance = new ThreadNotifier;
-
- return s_instance;
-}
-
-void ThreadNotifier::start()
-{
- if(!m_active++)
- plugin_register("timer", (void *)tick);
-}
-
-void ThreadNotifier::stop()
-{
- --m_active;
-}
-
-void ThreadNotifier::notify(const Notification ¬if)
-{
- lock_guard<mutex> guard(m_mutex);
-
- m_queue.push(notif);
-}
-
-void ThreadNotifier::tick()
-{
- ThreadNotifier *instance = ThreadNotifier::get();
- instance->processQueue();
-
- // doing this in stop() would cause a use after free of m_mutex in processQueue
- if(!instance->m_active) {
- plugin_register("-timer", (void *)tick);
-
- delete s_instance;
- s_instance = nullptr;
- }
-}
-
-void ThreadNotifier::processQueue()
-{
- lock_guard<mutex> guard(m_mutex);
-
- while(!m_queue.empty()) {
- const auto &[task, state] = m_queue.front();
- task->setState(state);
- m_queue.pop();
- }
-}
diff --git a/src/thread.hpp b/src/thread.hpp
@@ -19,6 +19,7 @@
#define REAPACK_THREAD_HPP
#include "errors.hpp"
+#include "event.hpp"
#include <array>
#include <condition_variable>
@@ -29,7 +30,7 @@
#include <boost/signals2.hpp>
-class ThreadTask {
+class ThreadTask : public EventEmitter {
public:
enum State {
Idle,
@@ -49,7 +50,6 @@ public:
void exec(); // runs in the current thread
const std::string &summary() const { return m_summary; }
- void setState(State);
State state() const { return m_state; }
void setError(const ErrorInfo &err) { m_error = err; }
const ErrorInfo &error() { return m_error; }
@@ -62,6 +62,7 @@ public:
protected:
virtual bool run() = 0;
+ void eventHandler(Event) override;
void setSummary(const std::string &s) { m_summary = s; }
@@ -122,30 +123,4 @@ private:
VoidSignal m_onDone;
};
-// This singleton class receives state change notifications from a
-// worker thread and applies them in the main thread
-class ThreadNotifier {
- typedef std::pair<ThreadTask *, ThreadTask::State> Notification;
-
-public:
- static ThreadNotifier *get();
-
- void start();
- void stop();
-
- void notify(const Notification &);
-
-private:
- static ThreadNotifier *s_instance;
- static void tick();
-
- ThreadNotifier() : m_active(0) {}
- ~ThreadNotifier() = default;
- void processQueue();
-
- std::mutex m_mutex;
- size_t m_active;
- std::queue<Notification> m_queue;
-};
-
#endif
diff --git a/test/event.cpp b/test/event.cpp
@@ -0,0 +1,66 @@
+#include "helper.hpp"
+
+#include <event.hpp>
+
+#include <reaper_plugin_functions.h>
+
+static constexpr const char *M = "[event]";
+
+TEST_CASE("multiple EventEmitter registers a single timer", M) {
+ static std::vector<std::string> registered;
+
+ plugin_register = [](const char *n, void *) {
+ registered.push_back(n);
+ return 0;
+ };
+
+ {
+ struct : EventEmitter { void eventHandler(Event) {} } e1, e2;
+ REQUIRE(registered == std::vector<std::string>{"timer"});
+ }
+
+ REQUIRE(registered == std::vector<std::string>{"timer", "-timer"});
+}
+
+TEST_CASE("EventEmitter calls eventHandler from timer and in order", M) {
+ static void (*tick)() = nullptr;
+ plugin_register = [](const char *, void *c) { tick = (void(*)())c; return 0; };
+
+ struct : EventEmitter {
+ void eventHandler(Event e) { bucket.push_back(e); }
+ std::vector<Event> bucket;
+ } e;
+
+ e.emit(1);
+ e.emit(2);
+ e.emit(3);
+ CHECK(e.bucket.empty());
+ tick();
+ REQUIRE(e.bucket == std::vector<Event>{1, 2, 3});
+}
+
+TEST_CASE("events from deleted EventEmmiter are discarded", M) {
+ static void (*tick)() = nullptr;
+ plugin_register = [](const char *, void *c) { tick = (void(*)())c; return 0; };
+
+ struct Mock : EventEmitter { void eventHandler(Event) {} };
+
+ Mock a; // keep the timer alive
+
+ Mock *b = new Mock();
+ b->emit(0);
+ delete b;
+ memset((void *)b, 0, sizeof(Mock));
+
+ tick();
+}
+
+TEST_CASE("deleting EventEmmiter from eventHandler is safe", M) {
+ static void (*tick)() = nullptr;
+ plugin_register = [](const char *, void *c) { tick = (void(*)())c; return 0; };
+
+ struct Mock : EventEmitter { void eventHandler(Event) { delete this; } };
+
+ (new Mock())->emit(0); // only one event!
+ tick();
+}
