-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Don't rename main thread at process level #17038
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
5a2736e
to
98b6d19
Compare
Set only the internal name. Fixes bitcoin#17036 for both `bitcoind` and `bitcoin-qt`.
0979347
to
07e4bdb
Compare
Ugh, I suppose this was added to set the internal name for logging. Pushed an alternative approach. |
Concept ACK |
Tested ACK 07e4bdb, |
Thanks for this; I tripped on this recently and wondered what was the issue. Testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 07e4bdb killall bitcoind
shuts down bitcoind mainnet/testnet/regtest, killall bitcoin-qt
shuts down ./src/qt/bitcoin-qt
, tests pass, very light code review. Good idea to add the @note
warning. Thanks!
07e4bdb Don't rename main thread at process level (Wladimir J. van der Laan) Pull request description: Set only the internal name for the main threads. Fixes #17036 for both `bitcoind` and `bitcoin-qt`. After this, e.g. `killall` works again for either. ACKs for top commit: promag: Tested ACK 07e4bdb, `killall bitcoind` and `killall bitcoin-qt` now just works! jonatack: ACK 07e4bdb `killall bitcoind` shuts down bitcoind mainnet/testnet/regtest, `killall bitcoin-qt` shuts down `./src/qt/bitcoin-qt`, tests pass, very light code review. Good idea to add the `@note` warning. Thanks! Tree-SHA512: 8f310ae646c83a02de7cc6869aa9aca1d53613d8fb762d05e3dfa52e17ca82abeb99044564cf7ba45b3c4b320e65bf8315d0e8834a9e696f097be5af638c6fd9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 07e4bdb, I have tested the code on Linux Mint 19.2: killall {bitcoind|bitcoin-qt}
works as expected (even with bash completion).
Also, ps
outputs for bitcoin-qt
(as example) with this PR:
$ ps
PID TTY TIME CMD
3137 pts/0 00:00:00 bash
17072 pts/0 00:00:07 bitcoin-qt
17100 pts/0 00:00:00 ps
$ ps -T
PID SPID TTY TIME CMD
3137 3137 pts/0 00:00:00 bash
17072 17072 pts/0 00:00:00 bitcoin-qt
17072 17073 pts/0 00:00:00 QXcbEventReader
17072 17074 pts/0 00:00:00 bitcoin:disk$0
17072 17075 pts/0 00:00:00 QDBusConnection
17072 17076 pts/0 00:00:00 gmain
17072 17077 pts/0 00:00:00 gdbus
17072 17078 pts/0 00:00:07 b-qt-init
17072 17079 pts/0 00:00:00 b-scriptch.0
17072 17080 pts/0 00:00:00 b-scriptch.1
17072 17081 pts/0 00:00:00 b-scriptch.2
17072 17082 pts/0 00:00:00 b-scheduler
17072 17083 pts/0 00:00:00 b-http
17072 17084 pts/0 00:00:00 b-httpworker.0
17072 17085 pts/0 00:00:00 b-httpworker.1
17072 17086 pts/0 00:00:00 b-httpworker.2
17072 17087 pts/0 00:00:00 b-httpworker.3
17072 17088 pts/0 00:00:00 b-qt-init
17072 17090 pts/0 00:00:00 b-torcontrol
17072 17091 pts/0 00:00:00 b-upnp
17072 17092 pts/0 00:00:00 b-net
17072 17093 pts/0 00:00:00 b-dnsseed
17072 17094 pts/0 00:00:00 b-addcon
17072 17095 pts/0 00:00:00 b-opencon
17072 17096 pts/0 00:00:00 b-msghand
17072 17097 pts/0 00:00:00 QThread
17072 17098 pts/0 00:00:00 QThread
17105 17105 pts/0 00:00:00 ps
ACK |
07e4bdb Don't rename main thread at process level (Wladimir J. van der Laan) Pull request description: Set only the internal name for the main threads. Fixes bitcoin#17036 for both `bitcoind` and `bitcoin-qt`. After this, e.g. `killall` works again for either. ACKs for top commit: promag: Tested ACK 07e4bdb, `killall bitcoind` and `killall bitcoin-qt` now just works! jonatack: ACK 07e4bdb `killall bitcoind` shuts down bitcoind mainnet/testnet/regtest, `killall bitcoin-qt` shuts down `./src/qt/bitcoin-qt`, tests pass, very light code review. Good idea to add the `@note` warning. Thanks! Tree-SHA512: 8f310ae646c83a02de7cc6869aa9aca1d53613d8fb762d05e3dfa52e17ca82abeb99044564cf7ba45b3c4b320e65bf8315d0e8834a9e696f097be5af638c6fd9
Set only the internal name. Fixes bitcoin#17036 for both `bitcoind` and `bitcoin-qt`. Github-Pull: bitcoin#17038 Rebased-From: 07e4bdb Tree-SHA512: ed6f1b95a23c4c7863982ee6972429be5af0702ea93f0f17d32d2ef4b01446b1c0528eeadc45289609eda5c02ea68b3d722b8ecdfdf4fff4b02592c2188cc0a0
Set only the internal name. Fixes bitcoin#17036 for both `bitcoind` and `bitcoin-qt`. Github-Pull: bitcoin#17038 Rebased-From: 07e4bdb Tree-SHA512: ed6f1b95a23c4c7863982ee6972429be5af0702ea93f0f17d32d2ef4b01446b1c0528eeadc45289609eda5c02ea68b3d722b8ecdfdf4fff4b02592c2188cc0a0
Set only the internal name. Fixes bitcoin#17036 for both `bitcoind` and `bitcoin-qt`. Github-Pull: bitcoin#17038 Rebased-From: 07e4bdb Tree-SHA512: ed6f1b95a23c4c7863982ee6972429be5af0702ea93f0f17d32d2ef4b01446b1c0528eeadc45289609eda5c02ea68b3d722b8ecdfdf4fff4b02592c2188cc0a0
Summary: ``` Set only the internal name. Fixes #17036 for both `bitcoind` and `bitcoin-qt`. ``` This fixes Unix tools like `ps` or `pkill` that use the process name (called `bitcoin-init` since D5540). Backport of core [[bitcoin/bitcoin#17038 | PR17038]]. Depends on D5540. Test Plan: ninja check bitcoind -daemon ps -A | grep bitcoind # should return our bitcoind instance sudo killall bitcoind ps -A | grep bitcoind # should return nothing, kill successful Reviewers: #bitcoin_abc, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D5544
Thread names in logs and deadlock debug tools See merge request bitcoin-cash-node/bitcoin-cash-node!834 This is a backport of [PR15849](bitcoin/bitcoin#15849) (Thread names in logs and deadlock debug tools). See original PR for benchmarks. Also includes some smaller follow-up PR's: * [PR15968](bitcoin/bitcoin#15968) - Fix portability issue with pthreads * [PR16984](bitcoin/bitcoin#16984) - util: Make thread names shorter * [PR17038](bitcoin/bitcoin#17038) - Don't rename main thread at process level ## Test plan New tests: `ninja check` Run with `-logthreadnames`, observe lines are prefixed with thread ``` 2020-10-28T11:39:16Z [init] init message: Done loading 2020-10-28T11:39:16Z [opencon] opencon thread start 2020-10-28T11:39:16Z [dnsseed] 0 addresses found from DNS seeds 2020-10-28T11:39:16Z [msghand] msghand thread start 2020-10-28T11:39:16Z [dnsseed] dnsseed thread exit ``` Compile with lockorder debugging enabled: `cmake -GNinja -DCMAKE_BUILD_TYPE=Debug ..`
07e4bdb Don't rename main thread at process level (Wladimir J. van der Laan) Pull request description: Set only the internal name for the main threads. Fixes bitcoin#17036 for both `bitcoind` and `bitcoin-qt`. After this, e.g. `killall` works again for either. ACKs for top commit: promag: Tested ACK 07e4bdb, `killall bitcoind` and `killall bitcoin-qt` now just works! jonatack: ACK 07e4bdb `killall bitcoind` shuts down bitcoind mainnet/testnet/regtest, `killall bitcoin-qt` shuts down `./src/qt/bitcoin-qt`, tests pass, very light code review. Good idea to add the `@note` warning. Thanks! Tree-SHA512: 8f310ae646c83a02de7cc6869aa9aca1d53613d8fb762d05e3dfa52e17ca82abeb99044564cf7ba45b3c4b320e65bf8315d0e8834a9e696f097be5af638c6fd9
07e4bdb Don't rename main thread at process level (Wladimir J. van der Laan) Pull request description: Set only the internal name for the main threads. Fixes bitcoin#17036 for both `bitcoind` and `bitcoin-qt`. After this, e.g. `killall` works again for either. ACKs for top commit: promag: Tested ACK 07e4bdb, `killall bitcoind` and `killall bitcoin-qt` now just works! jonatack: ACK 07e4bdb `killall bitcoind` shuts down bitcoind mainnet/testnet/regtest, `killall bitcoin-qt` shuts down `./src/qt/bitcoin-qt`, tests pass, very light code review. Good idea to add the `@note` warning. Thanks! Tree-SHA512: 8f310ae646c83a02de7cc6869aa9aca1d53613d8fb762d05e3dfa52e17ca82abeb99044564cf7ba45b3c4b320e65bf8315d0e8834a9e696f097be5af638c6fd9
Set only the internal name. Fixes bitcoin#17036 for both `bitcoind` and `bitcoin-qt`. Github-Pull: bitcoin#17038 Rebased-From: 07e4bdb Tree-SHA512: ed6f1b95a23c4c7863982ee6972429be5af0702ea93f0f17d32d2ef4b01446b1c0528eeadc45289609eda5c02ea68b3d722b8ecdfdf4fff4b02592c2188cc0a0
Set only the internal name for the main threads.
Fixes #17036 for both
bitcoind
andbitcoin-qt
.After this, e.g.
killall
works again for either.