From 519957bd590cffa0d9741fdff6363b5cc0030acd Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Mon, 18 Nov 2024 18:37:24 -0800 Subject: [PATCH] unnamed threads: Obliterate Ever read gdb output and you just kinda get a headache because you have to infer what a thread is by reading the stack trace? It's not hard, but we could also just never have to do that again, which is also not hard. Sample: (gdb) info thr Id Target Id Frame * 1 LWP 3719283 "nix-daemon" 0x00007e558587da0f in accept () from target:/nix/store/c10zhkbp6jmyh0xc5kd123ga8yy2p4hk-glibc-2.39-52/lib/libc.so.6 2 LWP 3719284 "signal handler" 0x00007e55857b2bea in sigtimedwait () from target:/nix/store/c10zhkbp6jmyh0xc5kd123ga8yy2p4hk-glibc-2.39-52/lib/libc.so.6 The API design for this is forced by the macOS pthread_setname_np only being able to change the current thread's name, but if we just conform everything to that, it works everywhere. Change-Id: I2b1d6ed41e3c94170cb0b4e73ad66f239ebd9c88 --- doc/manual/rl-next/thread-names.md | 17 +++++++++++++++++ src/libmain/progress-bar.cc | 2 ++ src/libstore/binary-cache-store.cc | 2 +- src/libstore/build/local-derivation-goal.cc | 3 +++ src/libstore/filetransfer.cc | 6 +++++- src/libstore/gc.cc | 8 +++++++- src/libstore/misc.cc | 2 +- src/libstore/remote-store.cc | 2 ++ src/libstore/store-api.cc | 6 +++--- src/libutil/meson.build | 2 ++ src/libutil/monitor-fd.hh | 4 ++-- src/libutil/signals.cc | 2 ++ src/libutil/thread-name.cc | 20 ++++++++++++++++++++ src/libutil/thread-name.hh | 12 ++++++++++++ src/libutil/thread-pool.cc | 9 ++++++--- src/libutil/thread-pool.hh | 4 +++- src/nix/sigs.cc | 2 +- src/nix/verify.cc | 2 +- tests/unit/libstore/filetransfer.cc | 3 +++ 19 files changed, 93 insertions(+), 15 deletions(-) create mode 100644 doc/manual/rl-next/thread-names.md create mode 100644 src/libutil/thread-name.cc create mode 100644 src/libutil/thread-name.hh diff --git a/doc/manual/rl-next/thread-names.md b/doc/manual/rl-next/thread-names.md new file mode 100644 index 000000000..e578be4c4 --- /dev/null +++ b/doc/manual/rl-next/thread-names.md @@ -0,0 +1,17 @@ +--- +synopsis: All Lix threads are named +cls: [2210] +category: Development +credits: [jade] +--- + +Lix now sets thread names on all of its secondary threads, which will make debugger usage slightly nicer and easier. + +``` +(gdb) info thr + Id Target Id Frame +* 1 LWP 3719283 "nix-daemon" 0x00007e558587da0f in accept () + from target:/nix/store/c10zhkbp6jmyh0xc5kd123ga8yy2p4hk-glibc-2.39-52/lib/libc.so.6 + 2 LWP 3719284 "signal handler" 0x00007e55857b2bea in sigtimedwait () + from target:/nix/store/c10zhkbp6jmyh0xc5kd123ga8yy2p4hk-glibc-2.39-52/lib/libc.so.6 +``` diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index 5c80f4011..d5a74a24e 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -4,6 +4,7 @@ #include "lix/libstore/names.hh" #include "lix/libutil/terminal.hh" #include "lix/libutil/strings.hh" +#include "lix/libutil/thread-name.hh" #include #include @@ -84,6 +85,7 @@ void ProgressBar::resume() if (state->paused > 0) return; // recursive pause, wait for the parents to resume too state->haveUpdate = true; updateThread = std::thread([&]() { + setCurrentThreadName("progress bar"); auto state(state_.lock()); auto nextWakeup = A_LONG_TIME; while (state->paused == 0) { diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index f57d84229..895629210 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -196,7 +196,7 @@ ref BinaryCacheStore::addToStoreCommon( if (narAccessor->stat(buildIdDir).type == FSAccessor::tDirectory) { - ThreadPool threadPool(25); + ThreadPool threadPool("write debuginfo pool", 25); auto doFile = [&](std::string member, std::string key, std::string target) { checkInterrupt(); diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 17b374546..dc86f1315 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -19,6 +19,7 @@ #include "lix/libutil/unix-domain-socket.hh" #include "lix/libutil/mount.hh" #include "lix/libutil/strings.hh" +#include "lix/libutil/thread-name.hh" #include #include @@ -1224,6 +1225,7 @@ void LocalDerivationGoal::startDaemon() chownToBuilder(socketPath); daemonThread = std::thread([this, store]() { + setCurrentThreadName("recursive nix daemon"); while (true) { @@ -1244,6 +1246,7 @@ void LocalDerivationGoal::startDaemon() debug("received daemon connection"); auto workerThread = std::thread([store, remote{std::move(remote)}]() { + setCurrentThreadName("recursive nix worker"); FdSource from(remote.get()); FdSink to(remote.get()); try { diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 68ead1d82..9627ed01c 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -5,6 +5,7 @@ #include "lix/libstore/s3.hh" #include "lix/libutil/signals.hh" #include "lix/libutil/strings.hh" +#include "lix/libutil/thread-name.hh" #include #include @@ -454,7 +455,10 @@ struct curlFileTransfer : public FileTransfer curl_multi_setopt(curlm.get(), CURLMOPT_MAX_TOTAL_CONNECTIONS, fileTransferSettings.httpConnections.get()); - workerThread = std::thread([&]() { workerThreadEntry(); }); + workerThread = std::thread([&]() { + setCurrentThreadName("curlFileTransfer worker"); + workerThreadEntry(); + }); } ~curlFileTransfer() diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 18a7e6026..b3f5250f7 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -6,6 +6,7 @@ #include "lix/libutil/finally.hh" #include "lix/libutil/unix-domain-socket.hh" #include "lix/libutil/strings.hh" +#include "lix/libutil/thread-name.hh" #include #include @@ -420,7 +421,10 @@ class GCOperation { throw SysError("making socket '%1%' non-blocking", socketPath); } - serverThread = std::thread([this]() { runServerThread(); }); + serverThread = std::thread([this]() { + setCurrentThreadName("gc server"); + runServerThread(); + }); } void addTempRoot(std::string rootHashPart) @@ -491,6 +495,7 @@ void GCOperation::runServerThread() /* Process the connection in a separate thread. */ auto fdClient_ = fdClient.get(); std::thread clientThread([&, fdClient = std::move(fdClient)]() { + setCurrentThreadName("gc server connection"); Finally cleanup([&]() { auto conn(connections.lock()); auto i = conn->find(fdClient.get()); @@ -900,6 +905,7 @@ void LocalStore::autoGC(bool sync) future = state->gcFuture = promise.get_future().share(); std::thread([promise{std::move(promise)}, this, avail, getAvail]() mutable { + setCurrentThreadName("auto gc"); try { diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 2dac16892..5aec1a8cf 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -86,7 +86,7 @@ void Store::queryMissing(const std::vector & targets, downloadSize_ = narSize_ = 0; // FIXME: make async. - ThreadPool pool(fileTransferSettings.httpConnections); + ThreadPool pool("queryMissing pool", fileTransferSettings.httpConnections); struct State { diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index a5f08df40..ab8ddc547 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -17,6 +17,7 @@ #include "lix/libutil/logging.hh" #include "lix/libstore/filetransfer.hh" #include "lix/libutil/strings.hh" +#include "lix/libutil/thread-name.hh" #include @@ -971,6 +972,7 @@ void RemoteStore::ConnectionHandle::withFramedSink(std::function(pool, storePathsToAdd, @@ -835,7 +835,7 @@ StorePathSet Store::queryValidPaths(const StorePathSet & paths, SubstituteFlag m Sync state_(State{paths.size(), StorePathSet()}); std::condition_variable wakeup; - ThreadPool pool; + ThreadPool pool{"queryValidPaths pool"}; auto doQuery = [&](const StorePath & path) { checkInterrupt(); @@ -1136,7 +1136,7 @@ std::map copyPaths( } auto pathsMap = copyPaths(srcStore, dstStore, storePaths, repair, checkSigs, substitute); - ThreadPool pool; + ThreadPool pool{"copyPaths pool"}; try { // Copy the realisation closure diff --git a/src/libutil/meson.build b/src/libutil/meson.build index b1b8742da..280849dff 100644 --- a/src/libutil/meson.build +++ b/src/libutil/meson.build @@ -38,6 +38,7 @@ libutil_sources = files( 'suggestions.cc', 'tarfile.cc', 'terminal.cc', + 'thread-name.cc', 'thread-pool.cc', 'unix-domain-socket.cc', 'url.cc', @@ -118,6 +119,7 @@ libutil_headers = files( 'sync.hh', 'tarfile.hh', 'terminal.hh', + 'thread-name.hh', 'thread-pool.hh', 'topo-sort.hh', 'types.hh', diff --git a/src/libutil/monitor-fd.hh b/src/libutil/monitor-fd.hh index 92ed9ff65..f160f0db7 100644 --- a/src/libutil/monitor-fd.hh +++ b/src/libutil/monitor-fd.hh @@ -4,15 +4,14 @@ #include #include -#include #include #include #include -#include #include "lix/libutil/error.hh" #include "lix/libutil/file-descriptor.hh" #include "lix/libutil/signals.hh" +#include "lix/libutil/thread-name.hh" namespace nix { @@ -34,6 +33,7 @@ public: auto &quit_ = this->quit; int terminateFd = terminatePipe.readSide.get(); thread = std::thread([fd, terminateFd, &quit_]() { + setCurrentThreadName("MonitorFdHup"); while (!quit_) { /* Wait indefinitely until a POLLHUP occurs. */ struct pollfd fds[2]; diff --git a/src/libutil/signals.cc b/src/libutil/signals.cc index 45edcaa65..d32a03b76 100644 --- a/src/libutil/signals.cc +++ b/src/libutil/signals.cc @@ -2,6 +2,7 @@ #include "lix/libutil/error.hh" #include "lix/libutil/sync.hh" #include "lix/libutil/terminal.hh" +#include "lix/libutil/thread-name.hh" #include #include @@ -49,6 +50,7 @@ static Sync _interruptCallbacks; static void signalHandlerThread(sigset_t set) { + setCurrentThreadName("signal handler"); while (true) { int signal = 0; sigwait(&set, &signal); diff --git a/src/libutil/thread-name.cc b/src/libutil/thread-name.cc new file mode 100644 index 000000000..85a3dcb61 --- /dev/null +++ b/src/libutil/thread-name.cc @@ -0,0 +1,20 @@ +#include +#if defined(__FreeBSD__) || defined(__OpenBSD__) +#include +#endif + +namespace nix { + +void setCurrentThreadName(const char * name) +{ + // https://stackoverflow.com/questions/2369738/how-to-set-the-name-of-a-thread-in-linux-pthreads/7989973 +#if defined(__linux__) + pthread_setname_np(pthread_self(), name); +#elif defined(__APPLE__) + pthread_setname_np(name); +#elif defined(__FreeBSD__) || defined(__OpenBSD__) + pthread_set_name_np(pthread_self(), name); +#endif +} + +} diff --git a/src/libutil/thread-name.hh b/src/libutil/thread-name.hh new file mode 100644 index 000000000..304cb1d35 --- /dev/null +++ b/src/libutil/thread-name.hh @@ -0,0 +1,12 @@ +#pragma once +///@file + +namespace nix { + +/** + * Sets the name of the current operating system thread for the benefit of + * debuggers. + */ +void setCurrentThreadName(const char * name); + +} diff --git a/src/libutil/thread-pool.cc b/src/libutil/thread-pool.cc index b4ea8363f..96d1cc2c0 100644 --- a/src/libutil/thread-pool.cc +++ b/src/libutil/thread-pool.cc @@ -1,11 +1,12 @@ #include "lix/libutil/thread-pool.hh" #include "lix/libutil/logging.hh" #include "lix/libutil/signals.hh" +#include "lix/libutil/thread-name.hh" namespace nix { -ThreadPool::ThreadPool(size_t _maxThreads) - : maxThreads(_maxThreads) +ThreadPool::ThreadPool(const char * name, size_t _maxThreads) + : maxThreads(_maxThreads), name(name) { if (!maxThreads) { maxThreads = std::thread::hardware_concurrency(); @@ -81,8 +82,10 @@ void ThreadPool::doWork(bool mainThread) { ReceiveInterrupts receiveInterrupts; - if (!mainThread) + if (!mainThread) { + setCurrentThreadName(this->name); interruptCheck = [&]() { return (bool) quit; }; + } bool didWork = false; std::exception_ptr exc; diff --git a/src/libutil/thread-pool.hh b/src/libutil/thread-pool.hh index 412290574..089fdd3f3 100644 --- a/src/libutil/thread-pool.hh +++ b/src/libutil/thread-pool.hh @@ -22,7 +22,7 @@ class ThreadPool { public: - ThreadPool(size_t maxThreads = 0); + ThreadPool(const char * name, size_t maxThreads = 0); ~ThreadPool(); @@ -56,6 +56,8 @@ private: size_t maxThreads; + const char * name; + struct State { std::queue pending; diff --git a/src/nix/sigs.cc b/src/nix/sigs.cc index ffeb9f8c2..d84b0e10a 100644 --- a/src/nix/sigs.cc +++ b/src/nix/sigs.cc @@ -38,7 +38,7 @@ struct CmdCopySigs : StorePathsCommand for (auto & s : substituterUris) substituters.push_back(openStore(s)); - ThreadPool pool; + ThreadPool pool{"CopySigs pool"}; std::string doneLabel = "done"; std::atomic added{0}; diff --git a/src/nix/verify.cc b/src/nix/verify.cc index 4890171bc..f51f00888 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -79,7 +79,7 @@ struct CmdVerify : StorePathsCommand act.progress(done, storePaths.size(), active, failed); }; - ThreadPool pool; + ThreadPool pool{"Verify pool"}; auto doPath = [&](const StorePath & storePath) { try { diff --git a/tests/unit/libstore/filetransfer.cc b/tests/unit/libstore/filetransfer.cc index c6cd0f9c6..e998aaaf7 100644 --- a/tests/unit/libstore/filetransfer.cc +++ b/tests/unit/libstore/filetransfer.cc @@ -1,5 +1,6 @@ #include "lix/libstore/filetransfer.hh" #include "lix/libutil/compression.hh" +#include "lix/libutil/thread-name.hh" #include #include @@ -90,6 +91,7 @@ serveHTTP(std::vector replies) std::thread( [replies, at{0}](AutoCloseFD socket, AutoCloseFD trigger) mutable { + setCurrentThreadName("test httpd server"); while (true) { pollfd pfds[2] = { { @@ -120,6 +122,7 @@ serveHTTP(std::vector replies) const auto & reply = replies[at++ % replies.size()]; std::thread([=, conn{std::move(conn)}] { + setCurrentThreadName("test httpd connection"); auto send = [&](std::string_view bit) { while (!bit.empty()) { auto written = ::send(conn.get(), bit.data(), bit.size(), MSG_NOSIGNAL);