Page MenuHomePhabricator

Thread names in logs and deadlock debug tools
ClosedPublic

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

Details

Reviewers
jasonbcox
Group Reviewers
Restricted Project
Commits
rABC3372a6e87684: Thread names in logs and deadlock debug tools
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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fabien created this revision.Mon, Mar 23, 15:32
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Mar 23, 15:32
teamcity edited the summary of this revision. (Show Details)Mon, Mar 23, 15:32

[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.Mon, Mar 23, 15:33
Fabien edited the summary of this revision. (Show Details)Mon, Mar 23, 15:36
Fabien edited the test plan for this revision. (Show Details)Mon, Mar 23, 16:28
Fabien requested review of this revision.Mon, Mar 23, 16:32

Gitian builds passed.

jasonbcox requested changes to this revision.Mon, Mar 23, 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.Mon, Mar 23, 17:00
Fabien updated this revision to Diff 17119.Mon, Mar 23, 17:13

Fix weird initializers

Fabien added inline comments.Mon, Mar 23, 17:13
src/sync.cpp
43 ↗(On Diff #17113)

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

Fabien updated this revision to Diff 17120.Mon, Mar 23, 17:15

Missing one.

jasonbcox accepted this revision.Mon, Mar 23, 17:34
This revision is now accepted and ready to land.Mon, Mar 23, 17:34
This revision was automatically updated to reflect the committed changes.