proxy: Name threads spawned by the event loop #324

ViniciusCestarii commented at 2:05 PM on July 31, 2026: contributor

Threads spawned by makeThread(), makePool() and the async cleanup thread inherit the name of the thread that created them, so tooling and log lines can't distinguish them from it. This PR rename them at creation.

bitcoin-node

vinicius@archlinux ~> ps -T -p $(pidof bitcoin-node) -o tid,comm
    TID COMMAND
  35689 bitcoin-node
  35690 b-scheduler
  35692 b-capnp-loop
  35694 b-http.00
  35695 b-http.01
  ...
  35740 b-opencon
  35741 b-msghand
  36017 capnp-async -- (Before this would be named "b-capnp-loop")

bitcoin-wallet

vinicius@archlinux ~> ps -T -p $(pidof bitcoin-wallet) -o tid,comm
    TID COMMAND
  35691 bitcoin-wallet
  35693 capnp-worker -- (Before this would be named "bitcoin-wallet")
  35742 b-schedqueue

Since ThreadName() reads the name back with pthread_getname_np(), log lines pick this up too:

2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Creating mp::ProxyServerBase<ipc::capnp::messages::Init, interfaces::Init> 0x7ff52c005430
2026-07-31T13:56:55Z ipc: {bitcoin-node-43644/b-capnp-loop-43647} IPC server: socket connected.
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} IPC server recv request  [#68](/bitcoin-core-multiprocess/68/) Init.construct$Params
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} IPC server send response [#68](/bitcoin-core-multiprocess/68/) Init.construct$Results
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} IPC server recv request  [#69](/bitcoin-core-multiprocess/69/) Init.makeRpc$Params
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} IPC server post request  [#69](/bitcoin-core-multiprocess/69/) {bitcoin-node-43644/capnp-worker-43826 (from bitcoin-cli-43824/bitcoin-cli-43824)}
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/capnp-worker-43826 (from bitcoin-cli-43824/bitcoin-cli-43824)} IPC server executing request [#69](/bitcoin-core-multiprocess/69/)
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Creating mp::ProxyClientBase<mp::Thread, capnp::Void> 0x7ff60403e928
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/capnp-worker-43826 (from bitcoin-cli-43824/bitcoin-cli-43824)} Creating mp::ProxyServerBase<ipc::capnp::messages::Rpc, interfaces::Rpc> 0x7ff52c0053b0
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Cleaning up mp::ProxyClientBase<mp::Thread, capnp::Void> 0x7ff60403e928
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Destroying mp::ProxyClientBase<mp::Thread, capnp::Void> 0x7ff60403e928
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} IPC server send response [#69](/bitcoin-core-multiprocess/69/) Init.makeRpc$Results
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} IPC server recv request  [#70](/bitcoin-core-multiprocess/70/) Rpc.executeRpc$Params
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} IPC server post request  [#70](/bitcoin-core-multiprocess/70/) {bitcoin-node-43644/capnp-worker-43826 (from bitcoin-cli-43824/bitcoin-cli-43824)}
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/capnp-worker-43826 (from bitcoin-cli-43824/bitcoin-cli-43824)} IPC server executing request [#70](/bitcoin-core-multiprocess/70/)
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Creating mp::ProxyClientBase<mp::Thread, capnp::Void> 0x7ff604017d48
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Creating mp::ProxyClientBase<mp::Thread, capnp::Void> 0x7ff60401b988
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Creating mp::ProxyClientBase<mp::Thread, capnp::Void> 0x7ff60401bf28
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/capnp-worker-43826 (from bitcoin-cli-43824/bitcoin-cli-43824)} IPC client send ActorCallback.call$Params
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/capnp-worker-43826 (from bitcoin-cli-43824/bitcoin-cli-43824)} IPC client recv ActorCallback.call$Results
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Cleaning up mp::ProxyClientBase<mp::Thread, capnp::Void> 0x7ff604017d48
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Destroying mp::ProxyClientBase<mp::Thread, capnp::Void> 0x7ff604017d48
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} IPC server send response [#70](/bitcoin-core-multiprocess/70/) Rpc.executeRpc$Results
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} IPC server destroy mp::ProxyServer<ipc::capnp::messages::Rpc>
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Cleaning up mp::ProxyServerBase<ipc::capnp::messages::Rpc, interfaces::Rpc> 0x7ff52c0053b0
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Destroying mp::ProxyServerBase<ipc::capnp::messages::Rpc, interfaces::Rpc> 0x7ff52c0053b0
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Cleaning up mp::ProxyClientBase<mp::Thread, capnp::Void> 0x7ff60401bf28
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Destroying mp::ProxyClientBase<mp::Thread, capnp::Void> 0x7ff60401bf28
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Cleaning up mp::ProxyClientBase<mp::Thread, capnp::Void> 0x7ff60401b988
2026-07-31T13:56:55Z [ipc] {bitcoin-node-43644/b-capnp-loop-43647} Destroying mp::ProxyClientBase<mp::Thread, capnp::Void> 0x7ff60401b988
2026-07-31T13:56:55Z ipc: {bitcoin-node-43644/b-capnp-loop-43647} IPC server: socket disconnected.

DrahtBot commented at 2:05 PM on July 31, 2026: none

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Stale ACK	enirox001

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#231 (Add windows support by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

in src/mp/util.cpp:83 in 61b6cd2577 outdated

  79 | @@ -74,6 +80,21 @@ template <std::size_t N>
  80 |  
  81 |  } // namespace
  82 |  
  83 | +void SetOsThreadName(const char* name)

ryanofsky commented at 9:36 PM on August 3, 2026:

In commit "proxy: Name threads spawned by the event loop" (61b6cd25776ea95a2d52910ba8799105d6cb838b)

I'd find it useful to have a comment here like // Copied from https://github.com/bitcoin/bitcoin/blob/d3e40af2597/src/util/threadnames.cpp#L21-L36 so this can be kept in sync if it makes sense.

ViniciusCestarii commented at 12:04 PM on August 4, 2026:

I agree that It makes sense. Done on 67af01544af579d843eff13104654e5f8c73f7c9

in src/mp/util.cpp:33 in 61b6cd2577

  28 | @@ -29,9 +29,15 @@
  29 |  #include <sys/syscall.h>
  30 |  #endif
  31 |  
  32 | -#ifdef HAVE_PTHREAD_GETTHREADID_NP
  33 | +
  34 | +#if (defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__DragonFly__))

ryanofsky commented at 9:48 PM on August 3, 2026:

In commit "proxy: Name threads spawned by the event loop" (61b6cd25776ea95a2d52910ba8799105d6cb838b)

Unclear why HAVE_PTHREAD_GETTHREADID_NP was removed here. It is still used another place below. Using HAVE_PTHREAD_GETTHREADID_NP does seem more reliable than checking for specific platforms.

ViniciusCestarii commented at 12:04 PM on August 4, 2026:

Ops yes I looked at it and it does make more sense to use HAVE_PTHREAD_GETTHREADID_NP. Done on 67af01544af579d843eff13104654e5f8c73f7c9

in src/mp/util.cpp:88 in 61b6cd2577

  79 | @@ -74,6 +80,21 @@ template <std::size_t N>
  80 |  
  81 |  } // namespace
  82 |  
  83 | +void SetOsThreadName(const char* name)
  84 | +{
  85 | +#if defined(PR_SET_NAME)
  86 | +    // Only the first 15 characters are used (16 - NUL terminator)
  87 | +    ::prctl(PR_SET_NAME, name, 0, 0, 0);
  88 | +#elif (defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__DragonFly__))

ryanofsky commented at 9:49 PM on August 3, 2026:

In commit "proxy: Name threads spawned by the event loop" (61b6cd25776ea95a2d52910ba8799105d6cb838b)

Probably better to use HAVE_PTHREAD_GETTHREADID_NP here

ViniciusCestarii commented at 12:04 PM on August 4, 2026:

True, done on 67af01544af579d843eff13104654e5f8c73f7c9

in src/mp/util.cpp:94 in 61b6cd2577 outdated

  89 | +    pthread_set_name_np(pthread_self(), name);
  90 | +#elif defined(__APPLE__)
  91 | +    pthread_setname_np(name);
  92 | +#else
  93 | +    // Prevent warnings for unused parameters...
  94 | +    (void)name;

ryanofsky commented at 9:50 PM on August 3, 2026:

In commit "proxy: Name threads spawned by the event loop" (61b6cd25776ea95a2d52910ba8799105d6cb838b)

Note for followup probably will want to extend this to windows.

ViniciusCestarii commented at 12:04 PM on August 4, 2026:

I agree, for Bitcoin Core it would be useful too.

ViniciusCestarii commented at 9:57 PM on August 4, 2026:

Opened for bitcoin core: https://github.com/bitcoin/bitcoin/pull/35884

ryanofsky commented at 9:55 PM on August 3, 2026: collaborator

Code review 61b6cd25776ea95a2d52910ba8799105d6cb838b. Looks good and thread names should be helpful, but I think the include check was probably better off using HAVE_PTHREAD_GETTHREADID_NP

proxy: Name threads spawned by the event loop 67af01544a

ViniciusCestarii force-pushed on Aug 4, 2026

ViniciusCestarii commented at 12:06 PM on August 4, 2026: contributor

Thanks for the review @ryanofsky! I forced push 67af01544af579d843eff13104654e5f8c73f7c9 so now it uses the macro HAVE_PTHREAD_GETTHREADID_NP instead.

enirox001 approved

enirox001 commented at 4:20 PM on August 6, 2026: contributor

ACK https://github.com/bitcoin-core/libmultiprocess/pull/324/changes/67af01544af579d843eff13104654e5f8c73f7c9#r3730102868

This looks good, and i can think of usecases for development and debugging,

But i wonder why no test covergae was added for this change? is there a reason for this? I think a simple test to test the behaviour of the method would be nice to have. Perhaps something like this?

KJ_TEST("IPC worker has OS thread name")
  {
      TestSetup setup;
      ProxyClient<messages::FooInterface>* foo = setup.client.get();
      foo->initThreadMap();
      setup.server->m_impl->m_fn = [] {};

      EventLoop& loop = *setup.server->m_context.connection->m_loop;
      std::promise<std::string> thread_name;
      auto thread_name_future = thread_name.get_future();

      loop.testing_hook_makethread_created = [&] {
          thread_name.set_value(ThreadName(""));
      };

      foo->callFnAsync();

      const std::string name = thread_name_future.get();
      KJ_EXPECT(name.find("/capnp-worker-") != std::string::npos,
      name);
  }

test: Cover OS thread names for worker, pool, and async threads 56cac0daad

ViniciusCestarii commented at 5:55 PM on August 6, 2026: contributor

But i wonder why no test covergae was added for this change? is there a reason for this? I think a simple test to test the behaviour of the method would be nice to have. Perhaps something like this?

Thanks for reviewing! Good point. tbh I thought it wound't be worth it but since this changes visible behavior (logs append OS thread name) it does make sense to add tests.

Pushed 56cac0daad2a9cf865d9461e73fa165c19dd3105: it adds tests under macro HAVE_PTHREAD_GETNAME_NP, so it only tests platforms that can get the thread OS name.

ViniciusCestarii commented at 6:22 PM on August 6, 2026: contributor

Just thinking out loud and out of scope here: it might be worth storing the thread name in a thread_local variable just like Bitcoin Core so the log string doesn't need to rely on platform specific get and set OS thread name and looks alike on every platform, while the OS thread naming itself stays as is for tools like ps/gdb, with support for other platforms added on demand.