fix deadbeef deadlock, restart after api test failure, add thread names

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

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(

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();

cpp/server/deadbeef/player_control.cpp

@@ -9,15 +9,17 @@ std::unique_ptr<PlayerState> PlayerImpl::queryPlayerState(ColumnsQuery* activeIt
 {
     auto state = std::make_unique<PlayerState>();
 
+    // 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<std::string> 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,17 +151,18 @@ 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();
     if (playlistIndex < 0)
         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;

cpp/server/deadbeef/player_misc.cpp

@@ -68,7 +68,7 @@ ColumnsQueryPtr PlayerImpl::createColumnsQuery(const std::vector<std::string>& c
 
 std::unique_ptr<WorkQueue> PlayerImpl::createWorkQueue()
 {
-    return std::make_unique<ThreadWorkQueue>();
+    return std::make_unique<ThreadWorkQueue>(MSRV_THREAD_NAME("control"));
 }
 
 void PlayerImpl::connect()
@@ -205,12 +205,12 @@ boost::unique_future<ArtworkResult> 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)

cpp/server/env_info.hpp.in

@@ -0,0 +1,4 @@
+#pragma once
+
+#cmakedefine HAVE_PTHREAD_SETNAME_NP
+#cmakedefine HAVE_PTHREAD_SET_NAME_NP

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); });

cpp/server/server_thread.cpp

@@ -1,5 +1,6 @@
 #include "server_thread.hpp"
 #include "log.hpp"
+#include "project_info.hpp"
 
 #include <boost/thread/reverse_lock.hpp>
 
@@ -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()

cpp/server/system.hpp

@@ -4,13 +4,19 @@
 
 #include <stdint.h>
 
+#if MSRV_OS_POSIX
+#include "env_info.hpp"
+#include <pthread.h>
+#endif
+
 #include <stdexcept>
 #include <type_traits>
 #include <vector>
 #include <string>
 #include <utility>
 
-namespace msrv {
+namespace msrv
+{
 
 template<typename Traits>
 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;

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<SetThreadDescriptionFunc>(
+        GetProcAddress(LoadLibraryA("kernelbase.dll"), "SetThreadDescription"));
+
+    if (setName)
+        setName(GetCurrentThread(), name);
+}
+
 const char* formatError(ErrorCode errorCode, char* buffer, size_t size) noexcept
 {
     auto ret = ::FormatMessageA(

cpp/server/work_queue.cpp

@@ -1,15 +1,21 @@
 #include "work_queue.hpp"
 #include "log.hpp"
+#include "system.hpp"
 
 #include <algorithm>
 
 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,15 +62,21 @@ void ThreadWorkQueue::run()
     }
 }
 
-ThreadPoolWorkQueue::ThreadPoolWorkQueue(size_t workers)
+ThreadPoolWorkQueue::ThreadPoolWorkQueue(size_t workers, ThreadName name)
 {
     assert(workers > 0);
 
     threads_.reserve(workers);
 
     for (size_t i = 0; i < workers; i++)
     {
-        threads_.emplace_back([this] { run(); });
+        threads_.emplace_back([this, name]
+        {
+            if (name)
+                setThreadName(name);
+
+            run();
+        });
     }
 }
 

cpp/server/work_queue.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "defines.hpp"
+#include "system.hpp"
 
 #include <thread>
 #include <mutex>
@@ -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;