Page MenuHomePhabricator

Thread names in logs and deadlock debug tools
ClosedPublic

Authored by Fabien on Mar 23 2020, 15:32.

Details

Summary

Backport of core PR15849.

The first commit disables thread_local on some platforms and is not ported:

  • The Darwin exclusion is no longer needed since D5514.
  • The MinGw exclusion does not seem necessary, at least with our version. The tests run fine on Windows, both 32 and 64 bits.

Also see D2219 which removes the thread_local check.

This introduces a number of bugs:

  • This is breaking the build for some *BSD.
  • This makes some Unix tools inefficient, like ps or killall.
  • Some thread names are truncated and cannot be differentiated.

The fixes will be ported in their own diff for readability.

Test Plan
ninja check
make check

Run the Gitian builds.

Run test_bitcoin.exe on Windows (32 bits) for sanity.
Builld and run the tests on OSX.

Start bitcoind with -logthreadnames=1 and check the debug log
contains the
thread names.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
PR15849
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 9937
Build 17733: Default Diff Build & Tests
Build 17732: arc lint + arc unit

Event Timeline

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

Fabien planned changes to this revision.Mar 23 2020, 15:33
Fabien requested review of this revision.Mar 23 2020, 16:32

Gitian builds passed.

jasonbcox requested changes to this revision.Mar 23 2020, 17:00
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/sync.cpp
43 ↗(On Diff #17113)

I'm surprised this compiles. All of these are supposed to be member initializers.

This revision now requires changes to proceed.Mar 23 2020, 17:00
src/sync.cpp
43 ↗(On Diff #17113)

oO good catch ! Indeed, it builds and works as expected, with both clang and gcc !

This revision is now accepted and ready to land.Mar 23 2020, 17:34