From 8649ea3d6fcbad110ecb668b8485cf4b8b45caba Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 10 Feb 2017 22:41:11 +0100 Subject: [PATCH] thread/Thread: use BoundMethod --- src/IOThread.cxx | 17 +++-- src/db/update/Service.cxx | 10 +-- src/db/update/Service.hxx | 1 - src/decoder/DecoderControl.cxx | 3 +- src/decoder/DecoderControl.hxx | 3 + src/decoder/DecoderThread.cxx | 38 +++++----- src/input/ThreadInputStream.cxx | 11 +-- src/input/ThreadInputStream.hxx | 2 +- .../plugins/SmbclientNeighborPlugin.cxx | 17 ++--- src/output/Init.cxx | 3 +- src/output/Internal.hxx | 1 - src/output/OutputThread.cxx | 11 +-- src/player/Control.cxx | 1 + src/player/Control.hxx | 3 + src/player/Thread.cxx | 74 +++++++++---------- src/thread/Thread.cxx | 11 +-- src/thread/Thread.hxx | 11 +-- test/run_output.cxx | 2 + 18 files changed, 99 insertions(+), 120 deletions(-) diff --git a/src/IOThread.cxx b/src/IOThread.cxx index a83c125a7..1e3e35074 100644 --- a/src/IOThread.cxx +++ b/src/IOThread.cxx @@ -27,12 +27,17 @@ #include -static struct { +static struct IOThread { Mutex mutex; Cond cond; EventLoop *loop; Thread thread; + + IOThread():thread(BIND_THIS_METHOD(Run)) {} + +private: + void Run() noexcept; } io; void @@ -44,15 +49,15 @@ io_thread_run(void) io.loop->Run(); } -static void -io_thread_func(gcc_unused void *arg) +inline void +IOThread::Run() noexcept { SetThreadName("io"); /* lock+unlock to synchronize with io_thread_start(), to be sure that io.thread is set */ - io.mutex.lock(); - io.mutex.unlock(); + mutex.lock(); + mutex.unlock(); io_thread_run(); } @@ -73,7 +78,7 @@ io_thread_start() assert(!io.thread.IsDefined()); const std::lock_guard protect(io.mutex); - io.thread.Start(io_thread_func, nullptr); + io.thread.Start(); } void diff --git a/src/db/update/Service.cxx b/src/db/update/Service.cxx index 2e7fe72da..099dc8104 100644 --- a/src/db/update/Service.cxx +++ b/src/db/update/Service.cxx @@ -43,6 +43,7 @@ UpdateService::UpdateService(EventLoop &_loop, SimpleDatabase &_db, :DeferredMonitor(_loop), db(_db), storage(_storage), listener(_listener), + update_thread(BIND_THIS_METHOD(Task)), update_task_id(0), walk(nullptr) { @@ -140,13 +141,6 @@ UpdateService::Task() DeferredMonitor::Schedule(); } -void -UpdateService::Task(void *ctx) -{ - UpdateService &service = *(UpdateService *)ctx; - return service.Task(); -} - void UpdateService::StartThread(UpdateQueueItem &&i) { @@ -158,7 +152,7 @@ UpdateService::StartThread(UpdateQueueItem &&i) next = std::move(i); walk = new UpdateWalk(GetEventLoop(), listener, *next.storage); - update_thread.Start(Task, this); + update_thread.Start(); FormatDebug(update_domain, "spawned thread for update job id %i", next.id); diff --git a/src/db/update/Service.hxx b/src/db/update/Service.hxx index 9bd2c344f..4ca38c52b 100644 --- a/src/db/update/Service.hxx +++ b/src/db/update/Service.hxx @@ -98,7 +98,6 @@ private: /* the update thread */ void Task(); - static void Task(void *ctx); void StartThread(UpdateQueueItem &&i); diff --git a/src/decoder/DecoderControl.cxx b/src/decoder/DecoderControl.cxx index 7a271b281..ef9b0bd5a 100644 --- a/src/decoder/DecoderControl.cxx +++ b/src/decoder/DecoderControl.cxx @@ -30,7 +30,8 @@ DecoderControl::DecoderControl(Mutex &_mutex, Cond &_client_cond, const AudioFormat _configured_audio_format, const ReplayGainConfig &_replay_gain_config) - :mutex(_mutex), client_cond(_client_cond), + :thread(BIND_THIS_METHOD(RunThread)), + mutex(_mutex), client_cond(_client_cond), configured_audio_format(_configured_audio_format), replay_gain_config(_replay_gain_config) {} diff --git a/src/decoder/DecoderControl.hxx b/src/decoder/DecoderControl.hxx index cc858c9a5..dfd8232e6 100644 --- a/src/decoder/DecoderControl.hxx +++ b/src/decoder/DecoderControl.hxx @@ -415,6 +415,9 @@ public: * mixramp_start/mixramp_end. */ void CycleMixRamp(); + +private: + void RunThread(); }; #endif diff --git a/src/decoder/DecoderThread.cxx b/src/decoder/DecoderThread.cxx index 7d4a91560..50dace3cc 100644 --- a/src/decoder/DecoderThread.cxx +++ b/src/decoder/DecoderThread.cxx @@ -513,30 +513,28 @@ try { dc.client_cond.signal(); } -static void -decoder_task(void *arg) +void +DecoderControl::RunThread() { - DecoderControl &dc = *(DecoderControl *)arg; - SetThreadName("decoder"); - const std::lock_guard protect(dc.mutex); + const std::lock_guard protect(mutex); do { - assert(dc.state == DecoderState::STOP || - dc.state == DecoderState::ERROR); + assert(state == DecoderState::STOP || + state == DecoderState::ERROR); - switch (dc.command) { + switch (command) { case DecoderCommand::START: - dc.CycleMixRamp(); - dc.replay_gain_prev_db = dc.replay_gain_db; - dc.replay_gain_db = 0; + CycleMixRamp(); + replay_gain_prev_db = replay_gain_db; + replay_gain_db = 0; - decoder_run(dc); + decoder_run(*this); - if (dc.state == DecoderState::ERROR) { + if (state == DecoderState::ERROR) { try { - std::rethrow_exception(dc.error); + std::rethrow_exception(error); } catch (const std::exception &e) { LogError(e); } catch (...) { @@ -552,20 +550,20 @@ decoder_task(void *arg) /* we need to clear the pipe here; usually the PlayerThread is responsible, but it is not aware that the decoder has finished */ - dc.pipe->Clear(*dc.buffer); + pipe->Clear(*buffer); - decoder_run(dc); + decoder_run(*this); break; case DecoderCommand::STOP: - dc.CommandFinishedLocked(); + CommandFinishedLocked(); break; case DecoderCommand::NONE: - dc.Wait(); + Wait(); break; } - } while (dc.command != DecoderCommand::NONE || !dc.quit); + } while (command != DecoderCommand::NONE || !quit); } void @@ -574,5 +572,5 @@ decoder_thread_start(DecoderControl &dc) assert(!dc.thread.IsDefined()); dc.quit = false; - dc.thread.Start(decoder_task, &dc); + dc.thread.Start(); } diff --git a/src/input/ThreadInputStream.cxx b/src/input/ThreadInputStream.cxx index 2463c7df4..88251a70a 100644 --- a/src/input/ThreadInputStream.cxx +++ b/src/input/ThreadInputStream.cxx @@ -54,10 +54,10 @@ ThreadInputStream::Start() assert(p != nullptr); buffer = new CircularBuffer((uint8_t *)p, buffer_size); - thread.Start(ThreadFunc, this); + thread.Start(); } -inline void +void ThreadInputStream::ThreadFunc() { FormatThreadName("input:%s", plugin); @@ -107,13 +107,6 @@ ThreadInputStream::ThreadFunc() Close(); } -void -ThreadInputStream::ThreadFunc(void *ctx) -{ - ThreadInputStream &tis = *(ThreadInputStream *)ctx; - tis.ThreadFunc(); -} - void ThreadInputStream::Check() { diff --git a/src/input/ThreadInputStream.hxx b/src/input/ThreadInputStream.hxx index 021bf6e03..046e0821c 100644 --- a/src/input/ThreadInputStream.hxx +++ b/src/input/ThreadInputStream.hxx @@ -73,6 +73,7 @@ public: size_t _buffer_size) :InputStream(_uri, _mutex, _cond), plugin(_plugin), + thread(BIND_THIS_METHOD(ThreadFunc)), buffer_size(_buffer_size) {} virtual ~ThreadInputStream(); @@ -138,7 +139,6 @@ protected: private: void ThreadFunc(); - static void ThreadFunc(void *ctx); }; #endif diff --git a/src/neighbor/plugins/SmbclientNeighborPlugin.cxx b/src/neighbor/plugins/SmbclientNeighborPlugin.cxx index 0b831a3c0..319e880b0 100644 --- a/src/neighbor/plugins/SmbclientNeighborPlugin.cxx +++ b/src/neighbor/plugins/SmbclientNeighborPlugin.cxx @@ -69,7 +69,8 @@ class SmbclientNeighborExplorer final : public NeighborExplorer { public: SmbclientNeighborExplorer(NeighborListener &_listener) - :NeighborExplorer(_listener) {} + :NeighborExplorer(_listener), + thread(BIND_THIS_METHOD(ThreadFunc)) {} /* virtual methods from class NeighborExplorer */ void Open() override; @@ -79,14 +80,13 @@ public: private: void Run(); void ThreadFunc(); - static void ThreadFunc(void *ctx); }; void SmbclientNeighborExplorer::Open() { quit = false; - thread.Start(ThreadFunc, this); + thread.Start(); } void @@ -239,6 +239,8 @@ SmbclientNeighborExplorer::Run() inline void SmbclientNeighborExplorer::ThreadFunc() { + SetThreadName("smbclient"); + mutex.lock(); while (!quit) { @@ -257,15 +259,6 @@ SmbclientNeighborExplorer::ThreadFunc() mutex.unlock(); } -void -SmbclientNeighborExplorer::ThreadFunc(void *ctx) -{ - SetThreadName("smbclient"); - - SmbclientNeighborExplorer &e = *(SmbclientNeighborExplorer *)ctx; - e.ThreadFunc(); -} - static NeighborExplorer * smbclient_neighbor_create(gcc_unused EventLoop &loop, NeighborListener &listener, diff --git a/src/output/Init.cxx b/src/output/Init.cxx index 453200dc9..34b53d916 100644 --- a/src/output/Init.cxx +++ b/src/output/Init.cxx @@ -51,7 +51,8 @@ AudioOutput::AudioOutput(const AudioOutputPlugin &_plugin, const ConfigBlock &block) - :plugin(_plugin) + :plugin(_plugin), + thread(BIND_THIS_METHOD(Task)) { assert(plugin.finish != nullptr); assert(plugin.open != nullptr); diff --git a/src/output/Internal.hxx b/src/output/Internal.hxx index 75355e8b9..4d23b8232 100644 --- a/src/output/Internal.hxx +++ b/src/output/Internal.hxx @@ -515,7 +515,6 @@ private: * The OutputThread. */ void Task(); - static void Task(void *arg); }; /** diff --git a/src/output/OutputThread.cxx b/src/output/OutputThread.cxx index d3f9c2bb8..91d61f576 100644 --- a/src/output/OutputThread.cxx +++ b/src/output/OutputThread.cxx @@ -396,7 +396,7 @@ AudioOutput::Pause() pause = false; } -inline void +void AudioOutput::Task() { FormatThreadName("output:%s", name); @@ -512,17 +512,10 @@ AudioOutput::Task() } } -void -AudioOutput::Task(void *arg) -{ - AudioOutput *ao = (AudioOutput *)arg; - ao->Task(); -} - void AudioOutput::StartThread() { assert(command == Command::NONE); - thread.Start(Task, this); + thread.Start(); } diff --git a/src/player/Control.cxx b/src/player/Control.cxx index b6c3688f4..013b57bf5 100644 --- a/src/player/Control.cxx +++ b/src/player/Control.cxx @@ -37,6 +37,7 @@ PlayerControl::PlayerControl(PlayerListener &_listener, buffer_chunks(_buffer_chunks), buffered_before_play(_buffered_before_play), configured_audio_format(_configured_audio_format), + thread(BIND_THIS_METHOD(RunThread)), replay_gain_config(_replay_gain_config) { } diff --git a/src/player/Control.hxx b/src/player/Control.hxx index e5023f0a9..39e24e4f8 100644 --- a/src/player/Control.hxx +++ b/src/player/Control.hxx @@ -537,6 +537,9 @@ public: void ApplyEnabled() override { LockUpdateAudio(); } + +private: + void RunThread(); }; #endif diff --git a/src/player/Thread.cxx b/src/player/Thread.cxx index 913e541e1..58883307d 100644 --- a/src/player/Thread.cxx +++ b/src/player/Thread.cxx @@ -1147,91 +1147,89 @@ do_play(PlayerControl &pc, DecoderControl &dc, player.Run(); } -static void -player_task(void *arg) +void +PlayerControl::RunThread() { - PlayerControl &pc = *(PlayerControl *)arg; - SetThreadName("player"); - DecoderControl dc(pc.mutex, pc.cond, - pc.configured_audio_format, - pc.replay_gain_config); + DecoderControl dc(mutex, cond, + configured_audio_format, + replay_gain_config); decoder_thread_start(dc); - MusicBuffer buffer(pc.buffer_chunks); + MusicBuffer buffer(buffer_chunks); - pc.Lock(); + Lock(); while (1) { - switch (pc.command) { + switch (command) { case PlayerCommand::SEEK: case PlayerCommand::QUEUE: - assert(pc.next_song != nullptr); + assert(next_song != nullptr); - pc.Unlock(); - do_play(pc, dc, buffer); - pc.listener.OnPlayerSync(); - pc.Lock(); + Unlock(); + do_play(*this, dc, buffer); + listener.OnPlayerSync(); + Lock(); break; case PlayerCommand::STOP: - pc.Unlock(); - pc.outputs.Cancel(); - pc.Lock(); + Unlock(); + outputs.Cancel(); + Lock(); /* fall through */ case PlayerCommand::PAUSE: - delete pc.next_song; - pc.next_song = nullptr; + delete next_song; + next_song = nullptr; - pc.CommandFinished(); + CommandFinished(); break; case PlayerCommand::CLOSE_AUDIO: - pc.Unlock(); + Unlock(); - pc.outputs.Release(); + outputs.Release(); - pc.Lock(); - pc.CommandFinished(); + Lock(); + CommandFinished(); assert(buffer.IsEmptyUnsafe()); break; case PlayerCommand::UPDATE_AUDIO: - pc.Unlock(); - pc.outputs.EnableDisable(); - pc.Lock(); - pc.CommandFinished(); + Unlock(); + outputs.EnableDisable(); + Lock(); + CommandFinished(); break; case PlayerCommand::EXIT: - pc.Unlock(); + Unlock(); dc.Quit(); - pc.outputs.Close(); + outputs.Close(); - pc.LockCommandFinished(); + LockCommandFinished(); return; case PlayerCommand::CANCEL: - delete pc.next_song; - pc.next_song = nullptr; + delete next_song; + next_song = nullptr; - pc.CommandFinished(); + CommandFinished(); break; case PlayerCommand::REFRESH: /* no-op when not playing */ - pc.CommandFinished(); + CommandFinished(); break; case PlayerCommand::NONE: - pc.Wait(); + Wait(); break; } } @@ -1242,5 +1240,5 @@ StartPlayerThread(PlayerControl &pc) { assert(!pc.thread.IsDefined()); - pc.thread.Start(player_task, &pc); + pc.thread.Start(); } diff --git a/src/thread/Thread.cxx b/src/thread/Thread.cxx index 16d68dc23..8d8a429d0 100644 --- a/src/thread/Thread.cxx +++ b/src/thread/Thread.cxx @@ -25,14 +25,11 @@ #include "java/Global.hxx" #endif -bool -Thread::Start(void (*_f)(void *ctx), void *_ctx) +void +Thread::Start() { assert(!IsDefined()); - f = _f; - ctx = _ctx; - #ifdef _WIN32 handle = ::CreateThread(nullptr, 0, ThreadProc, this, 0, &id); if (handle == nullptr) @@ -56,8 +53,6 @@ Thread::Start(void (*_f)(void *ctx), void *_ctx) creating = false; #endif #endif - - return true; } void @@ -89,7 +84,7 @@ Thread::Run() #endif #endif - f(ctx); + f(); #ifdef ANDROID Java::DetachCurrentThread(); diff --git a/src/thread/Thread.hxx b/src/thread/Thread.hxx index 9a58614a7..63f34f8e7 100644 --- a/src/thread/Thread.hxx +++ b/src/thread/Thread.hxx @@ -21,6 +21,7 @@ #define MPD_THREAD_HXX #include "check.h" +#include "util/BindMethod.hxx" #include "Compiler.h" #ifdef _WIN32 @@ -32,6 +33,9 @@ #include class Thread { + typedef BoundMethod Function; + const Function f; + #ifdef _WIN32 HANDLE handle = nullptr; DWORD id; @@ -49,11 +53,8 @@ class Thread { #endif #endif - void (*f)(void *ctx); - void *ctx; - public: - Thread() = default; + explicit Thread(Function _f):f(_f) {} Thread(const Thread &) = delete; @@ -89,7 +90,7 @@ public: #endif } - bool Start(void (*f)(void *ctx), void *ctx); + void Start(); void Join(); private: diff --git a/test/run_output.cxx b/test/run_output.cxx index 8af46211b..43272591c 100644 --- a/test/run_output.cxx +++ b/test/run_output.cxx @@ -44,6 +44,8 @@ #include #include +void AudioOutput::Task() {} + class DummyAudioOutputClient final : public AudioOutputClient { public: /* virtual methods from AudioOutputClient */