diff --git a/ChangeLog.md b/ChangeLog.md index 6086b333..9f05b393 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -1,6 +1,7 @@ # Changes in v0.11 (not released) ### DeaDBeeF - Provide universal .deb package to match `deadbeef-static_*.deb` +- Fix deadlock with streamer # Changes in v0.10 (released 2025-02-28) - Add ability to configure output device diff --git a/cpp/server/CMakeLists.txt b/cpp/server/CMakeLists.txt index ee408afc..29b628d4 100644 --- a/cpp/server/CMakeLists.txt +++ b/cpp/server/CMakeLists.txt @@ -1,5 +1,7 @@ set(CMAKE_CXX_STANDARD 17) +include(CheckSymbolExists) + if(CXX_GCC OR CXX_CLANG) add_cxx_compiler_flag(-Wall) add_cxx_compiler_flag(-Wextra) @@ -27,6 +29,12 @@ if(OS_POSIX) add_linker_flag(-Wl,--no-undefined) add_linker_flag(-Wl,--as-needed) + + check_symbol_exists(pthread_setname_np "pthread.h" HAVE_PTHREAD_SETNAME_NP) + + if(NOT HAVE_PTHREAD_SETNAME_NP) + check_symbol_exists(pthread_set_name_np "pthread.h" HAVE_PTHREAD_SET_NAME_NP) + endif() endif() if(OS_WINDOWS) @@ -51,6 +59,7 @@ find_package(Nljson REQUIRED) find_package(StringEncoders REQUIRED) find_package(ZLIB REQUIRED) +configure_file(env_info.hpp.in env_info.hpp) configure_file(project_info.hpp.in project_info.hpp) include_directories( diff --git a/cpp/server/deadbeef/player.hpp b/cpp/server/deadbeef/player.hpp index 6858f892..fcc9b7db 100644 --- a/cpp/server/deadbeef/player.hpp +++ b/cpp/server/deadbeef/player.hpp @@ -126,7 +126,7 @@ class PlayerImpl final : public Player using PlaylistItemSelector = DB_playItem_t* (*)(DB_playItem_t*, int); PlaybackState getPlaybackState(ddb_playItem_t* activeItem); - void queryActiveItem(ActiveItemInfo* info, ddb_playItem_t* activeItem, ColumnsQuery* query); + void queryActiveItem(ActiveItemInfo* info, ddb_playItem_t* activeItem, float activeItemPos, ColumnsQuery* query); void queryVolume(VolumeInfo* info); void queryInfo(PlayerInfo* info); bool checkOutputChanged(); diff --git a/cpp/server/deadbeef/player_control.cpp b/cpp/server/deadbeef/player_control.cpp index 6cb9e55d..45b78e84 100644 --- a/cpp/server/deadbeef/player_control.cpp +++ b/cpp/server/deadbeef/player_control.cpp @@ -9,15 +9,17 @@ std::unique_ptr PlayerImpl::queryPlayerState(ColumnsQuery* activeIt { auto state = std::make_unique(); + // Read position before obtaining playlist lock + PlaylistItemPtr activeItem(ddbApi->streamer_get_playing_track()); + auto activeItemPos = activeItem ? ddbApi->streamer_get_playpos() : -1.0f; + PlaylistLockGuard lock(playlistMutex_); playlists_.ensureInitialized(); - PlaylistItemPtr activeItem(ddbApi->streamer_get_playing_track()); - state->playbackState = getPlaybackState(activeItem.get()); queryInfo(&state->info); - queryActiveItem(&state->activeItem, activeItem.get(), activeItemQuery); + queryActiveItem(&state->activeItem, activeItem.get(), activeItemPos, activeItemQuery); queryVolume(&state->volume); queryOptions(state.get()); return state; @@ -51,7 +53,8 @@ PlaybackState PlayerImpl::getPlaybackState(ddb_playItem_t* activeItem) } } -void PlayerImpl::queryActiveItem(ActiveItemInfo* info, ddb_playItem_t* activeItem, ColumnsQuery* query) +void PlayerImpl::queryActiveItem( + ActiveItemInfo* info, ddb_playItem_t* activeItem, float activeItemPos, ColumnsQuery* query) { int playlistIndex = ddbApi->streamer_get_current_playlist(); @@ -64,13 +67,11 @@ void PlayerImpl::queryActiveItem(ActiveItemInfo* info, ddb_playItem_t* activeIte playlistId = playlists_.getId(playlist.get()); int32_t itemIndex = -1; - double itemPosition = -1.0; double itemDuration = -1.0; std::vector columns; if (activeItem) { - itemPosition = ddbApi->streamer_get_playpos(); itemDuration = ddbApi->pl_get_item_duration(activeItem); if (playlist) @@ -87,7 +88,7 @@ void PlayerImpl::queryActiveItem(ActiveItemInfo* info, ddb_playItem_t* activeIte info->playlistId = std::move(playlistId); info->playlistIndex = playlistIndex; info->index = itemIndex; - info->position = itemPosition; + info->position = activeItemPos; info->duration = itemDuration; info->columns = std::move(columns); } @@ -150,6 +151,11 @@ bool PlayerImpl::playNextBy(const std::string& expression, PlayerImpl::PlaylistI if (!format) throw InvalidRequestException("invalid format expression: " + expression); + PlaylistItemPtr activeItem(ddbApi->streamer_get_playing_track()); + + if (!activeItem) + return false; + PlaylistLockGuard lock(playlistMutex_); int playlistIndex = ddbApi->streamer_get_current_playlist(); @@ -157,10 +163,6 @@ bool PlayerImpl::playNextBy(const std::string& expression, PlayerImpl::PlaylistI return false; PlaylistPtr playlist(ddbApi->plt_get_for_idx(playlistIndex)); - PlaylistItemPtr activeItem(ddbApi->streamer_get_playing_track()); - - if (!activeItem) - return false; ddb_tf_context_t context{}; context._size = sizeof(context); @@ -249,8 +251,6 @@ void PlayerImpl::setMuted(Switch value) void PlayerImpl::seekAbsolute(double offsetSeconds) { - PlaylistLockGuard lock(playlistMutex_); - PlaylistItemPtr item(ddbApi->streamer_get_playing_track()); if (!item) return; @@ -260,8 +260,6 @@ void PlayerImpl::seekAbsolute(double offsetSeconds) void PlayerImpl::seekRelative(double offsetSeconds) { - PlaylistLockGuard lock(playlistMutex_); - PlaylistItemPtr item(ddbApi->streamer_get_playing_track()); if (!item) return; diff --git a/cpp/server/deadbeef/player_misc.cpp b/cpp/server/deadbeef/player_misc.cpp index 67c00d42..dab550f9 100644 --- a/cpp/server/deadbeef/player_misc.cpp +++ b/cpp/server/deadbeef/player_misc.cpp @@ -68,7 +68,7 @@ ColumnsQueryPtr PlayerImpl::createColumnsQuery(const std::vector& c std::unique_ptr PlayerImpl::createWorkQueue() { - return std::make_unique(); + return std::make_unique(MSRV_THREAD_NAME("control")); } void PlayerImpl::connect() @@ -205,12 +205,12 @@ boost::unique_future PlayerImpl::fetchCurrentArtwork() return boost::make_future(ArtworkResult()); } - PlaylistLockGuard lock(playlistMutex_); - PlaylistItemPtr item(ddbApi->streamer_get_playing_track()); if (!item) return boost::make_future(ArtworkResult()); + PlaylistLockGuard lock(playlistMutex_); + PlaylistPtr playlist; int playlistIndex = ddbApi->streamer_get_current_playlist(); if (playlistIndex >= 0) diff --git a/cpp/server/env_info.hpp.in b/cpp/server/env_info.hpp.in new file mode 100644 index 00000000..ccbb8e2f --- /dev/null +++ b/cpp/server/env_info.hpp.in @@ -0,0 +1,4 @@ +#pragma once + +#cmakedefine HAVE_PTHREAD_SETNAME_NP +#cmakedefine HAVE_PTHREAD_SET_NAME_NP diff --git a/cpp/server/server_host.cpp b/cpp/server/server_host.cpp index d4bb8173..f715281b 100644 --- a/cpp/server/server_host.cpp +++ b/cpp/server/server_host.cpp @@ -17,7 +17,7 @@ namespace msrv { ServerHost::ServerHost(Player* player) - : player_(player), utilityQueue_(8) + : player_(player), utilityQueue_(8, MSRV_THREAD_NAME("io")) { playerWorkQueue_ = player_->createWorkQueue(); player_->onEvents([this](PlayerEvents event) { handlePlayerEvents(event); }); diff --git a/cpp/server/server_thread.cpp b/cpp/server/server_thread.cpp index f626f6f9..c9fb0c02 100644 --- a/cpp/server/server_thread.cpp +++ b/cpp/server/server_thread.cpp @@ -1,5 +1,6 @@ #include "server_thread.hpp" #include "log.hpp" +#include "project_info.hpp" #include @@ -9,7 +10,10 @@ ServerThread::ServerThread(ServerReadyCallback readyCallback) : command_(Command::NONE), readyCallback_(std::move(readyCallback)) { - thread_ = std::thread([this] { run(); }); + thread_ = std::thread([this] { + setThreadName(MSRV_THREAD_NAME("server")); + run(); + }); } ServerThread::~ServerThread() diff --git a/cpp/server/system.hpp b/cpp/server/system.hpp index cafe367c..7dc8f0f0 100644 --- a/cpp/server/system.hpp +++ b/cpp/server/system.hpp @@ -4,13 +4,19 @@ #include +#if MSRV_OS_POSIX +#include "env_info.hpp" +#include +#endif + #include #include #include #include #include -namespace msrv { +namespace msrv +{ template class Handle @@ -86,6 +92,21 @@ class Handle #if MSRV_OS_POSIX +#define MSRV_THREAD_NAME(s) MSRV_PROJECT_ID "-" s + +typedef const char* ThreadName; + +inline void setThreadName(ThreadName name) +{ +#if defined(HAVE_PTHREAD_SETNAME_NP) + (void) pthread_setname_np(pthread_self(), name); +#elif defined(HAVE_PTHREAD_SET_NAME_NP) + (void) pthread_set_name_np(pthread_self(), name); +#else + (void)name; +#endif +} + struct PosixHandleTraits { using Type = int; @@ -113,6 +134,14 @@ inline ErrorCode lastSystemError() noexcept #if MSRV_OS_WINDOWS +#define MSRV_THREAD_NAME__(s) L ## s +#define MSRV_THREAD_NAME_(s) MSRV_THREAD_NAME__(s) +#define MSRV_THREAD_NAME(s) MSRV_THREAD_NAME_(MSRV_PROJECT_ID "-" s) + +typedef const wchar_t* ThreadName; + +void setThreadName(ThreadName name); + struct WindowsHandleTraits { typedef void* Type; diff --git a/cpp/server/system_windows.cpp b/cpp/server/system_windows.cpp index 44facf73..b11465bb 100644 --- a/cpp/server/system_windows.cpp +++ b/cpp/server/system_windows.cpp @@ -4,6 +4,17 @@ namespace msrv { +typedef HRESULT (WINAPI *SetThreadDescriptionFunc)(HANDLE thread, LPCWSTR name); + +void setThreadName(ThreadName name) +{ + static auto setName = reinterpret_cast( + GetProcAddress(LoadLibraryA("kernelbase.dll"), "SetThreadDescription")); + + if (setName) + setName(GetCurrentThread(), name); +} + const char* formatError(ErrorCode errorCode, char* buffer, size_t size) noexcept { auto ret = ::FormatMessageA( diff --git a/cpp/server/work_queue.cpp b/cpp/server/work_queue.cpp index 6c9844b4..2bf7c620 100644 --- a/cpp/server/work_queue.cpp +++ b/cpp/server/work_queue.cpp @@ -1,5 +1,6 @@ #include "work_queue.hpp" #include "log.hpp" +#include "system.hpp" #include @@ -7,9 +8,14 @@ namespace msrv { WorkQueue::~WorkQueue() = default; -ThreadWorkQueue::ThreadWorkQueue() +ThreadWorkQueue::ThreadWorkQueue(ThreadName name) { - thread_ = std::thread([this] { run(); }); + thread_ = std::thread([this, name] { + if (name) + setThreadName(name); + + run(); + }); } ThreadWorkQueue::~ThreadWorkQueue() @@ -56,7 +62,7 @@ void ThreadWorkQueue::run() } } -ThreadPoolWorkQueue::ThreadPoolWorkQueue(size_t workers) +ThreadPoolWorkQueue::ThreadPoolWorkQueue(size_t workers, ThreadName name) { assert(workers > 0); @@ -64,7 +70,13 @@ ThreadPoolWorkQueue::ThreadPoolWorkQueue(size_t workers) for (size_t i = 0; i < workers; i++) { - threads_.emplace_back([this] { run(); }); + threads_.emplace_back([this, name] + { + if (name) + setThreadName(name); + + run(); + }); } } diff --git a/cpp/server/work_queue.hpp b/cpp/server/work_queue.hpp index 4fd2bf82..59bcbedb 100644 --- a/cpp/server/work_queue.hpp +++ b/cpp/server/work_queue.hpp @@ -1,6 +1,7 @@ #pragma once #include "defines.hpp" +#include "system.hpp" #include #include @@ -29,7 +30,7 @@ class WorkQueue class ThreadWorkQueue : public WorkQueue { public: - ThreadWorkQueue(); + explicit ThreadWorkQueue(ThreadName name = nullptr); ~ThreadWorkQueue(); void enqueue(WorkCallback callback) override; @@ -48,7 +49,7 @@ class ThreadWorkQueue : public WorkQueue class ThreadPoolWorkQueue : public WorkQueue { public: - explicit ThreadPoolWorkQueue(size_t workers); + explicit ThreadPoolWorkQueue(size_t workers, ThreadName name = nullptr); ~ThreadPoolWorkQueue(); void enqueue(WorkCallback callback) override; diff --git a/js/api_tests/src/test_context.js b/js/api_tests/src/test_context.js index abd888e9..7d711cbb 100644 --- a/js/api_tests/src/test_context.js +++ b/js/api_tests/src/test_context.js @@ -17,9 +17,10 @@ export class TestContext this.tracks = tracks; this.outputConfigs = outputConfigs; this.options = null; + this.wantRestart = false; } - async beginSuite(options = {}) + initOptions(options) { const pluginSettings = Object.assign({}, this.config.pluginSettings, options.pluginSettings); @@ -33,22 +34,29 @@ export class TestContext }, options.resetOptions); - const axiosConfig = options.axiosConfig || null; - const environment = options.environment || null; + const { axiosConfig, environment } = options; - this.options = options = { + this.options = { pluginSettings, resetOptions, axiosConfig, environment }; + } - await this.player.start(options); + initClient() + { + this.client.handler.init(this.options.axiosConfig); + } - this.client.handler.init(axiosConfig); + async startPlayer() + { + await this.player.start(this.options); if (await this.client.waitUntilReady()) + { return; + } const logData = await this.player.getLog(); @@ -57,17 +65,50 @@ export class TestContext throw Error('Failed to reach API endpoint'); } + + async stopPlayer() + { + await this.player.stop(); + } + + async beginSuite(options = {}, reuseOptions = false) + { + this.wantRestart = false; + this.initOptions(options); + this.initClient(); + await this.startPlayer(); + } async endSuite() { this.options = null; - await this.player.stop(); + await this.stopPlayer(); } async beginTest() { - this.client.handler.init(this.options.axiosConfig); - await this.client.resetState(this.options.resetOptions); + if (this.wantRestart) + { + await this.stopPlayer(); + this.initClient(); + await this.startPlayer(); + this.wantRestart = false; + } + else + { + this.initClient(); + } + + try + { + await this.client.resetState(this.options.resetOptions); + } + catch (err) + { + console.log('failed to reset player state, next test will restart player'); + this.wantRestart = true; + throw err; + } } endTest()