Truncating thread name on linux
High Level Overview of Change
- Truncate/shorten long thread names on Linux to 15 chars as per limit.
- Warn/notify developer/user about the truncation in stderr. By default, notification is ON in Debug mode, OFF in Release. Can be overwritten with -DTRUNCATED_THREAD_NAME_LOGS=ON/OFF during cmake configure.
Context of Change
Fixing 5691. The setCurrentThreadName function does not warn or fail when provided with a string that is too long for the target platform. This results in the name being silently truncated by the operating system, which can make debugging difficult.
For example, on Linux, the thread name is limited to 15 characters (the underlying TASK_COMM_LEN is 16 bytes, including the null terminator). Passing a longer string should ideally trigger an assertion in debug builds or be handled in a way that notifies the developer.
Furthermore, given that in most rippled use cases we pass constant string literals, this check can also be performed at compile time to catch errors even earlier in the development process.
Type of Change
- [x] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Refactor (non-breaking change that only restructures code)
- [ ] Performance (increase or change in throughput and/or latency)
- [x] Tests (you added tests for code that already exists, or your new feature included in this PR)
- [ ] Documentation update
- [ ] Chore (no impact to binary, e.g.
.gitignore, formatting, dropping support for older tooling) - [ ] Release
API Impact
- [ ] Public API: New feature (new methods and/or new fields)
- [ ] Public API: Breaking change (in general, breaking changes should only impact the next api_version)
- [x]
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl) - [ ] Peer protocol change (must be backward compatible or bump the peer protocol version)
Made some changed as suggested. Please have a look. Just so you know, I managed to build and test this in all Linux, Windows and MacOs.
I think I still need to add support for Windows and MacOs. I only recently realised this needs to be done not only for Linux...
Made some changed as suggested. Please have a look. Just so you know, I managed to build and test this in all Linux, Windows and MacOs.
I think I still need to add support for Windows and MacOs. I only recently realised this needs to be done not only for Linux...
I don't think macos and windows enforce a length limit on thread names so we only have to make sure the logic only applies to linux, the code looks good otherwise.
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 78.6%. Comparing base (21c0223) to head (9002d3d).
:warning: Report is 8 commits behind head on develop.
Additional details and impacted files
@@ Coverage Diff @@
## develop #5758 +/- ##
=======================================
Coverage 78.6% 78.6%
=======================================
Files 818 819 +1
Lines 68953 68961 +8
Branches 8247 8239 -8
=======================================
+ Hits 54167 54186 +19
+ Misses 14786 14775 -11
| Files with missing lines | Coverage Δ | |
|---|---|---|
| include/xrpl/beast/core/CurrentThreadName.h | 100.0% <100.0%> (ø) |
|
| src/libxrpl/beast/core/CurrentThreadName.cpp | 100.0% <100.0%> (ø) |
|
| src/libxrpl/resource/ResourceManager.cpp | 75.0% <100.0%> (ø) |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Seems that we've got some build errors. Please could you run the pre-commit script and resolve the conflicts in RippledSettings.cmake when you have a chance? Thank you!
@janibakin would you be able to resolve the conflicts & use the pre-commit hooks to format the code?
@bthomee Hi, sure, will do. I'm quite busy, but I'll come to this during the weekend hopefully.
@bthomee done, rebased and run pre-commit hooks to format.