Add NativeThreadInfo and blend in Message#3333
Closed
osmeest wants to merge 4 commits intopocoproject:mainfrom
Closed
Add NativeThreadInfo and blend in Message#3333osmeest wants to merge 4 commits intopocoproject:mainfrom
osmeest wants to merge 4 commits intopocoproject:mainfrom
Conversation
osmeest
commented
Jun 29, 2021
- test with PatternFormatter
+ use in Message + test with PatternFormatter
Member
|
few things missing here:
|
bas524
reviewed
Jun 6, 2023
Foundation/src/Message.cpp
Outdated
| _tid = pThread->id(); | ||
| _thread = pThread->name(); | ||
| } | ||
| else { |
Contributor
There was a problem hiding this comment.
it should be formatted as
}
else
{
aleks-f
approved these changes
Dec 22, 2025
aleks-f
requested changes
Dec 22, 2025
Member
aleks-f
left a comment
There was a problem hiding this comment.
yeah, we have this problem here again:
In file included from /home/runner/work/poco/poco/Foundation/src/NativeThreadInfo.cpp:26:
/home/runner/work/poco/poco/Foundation/src/NativeThreadInfo_POSIX.cpp:39:11: error: use of undeclared identifier 'pthread_getname_np'
39 | int rc = pthread_getname_np(_thread, &result[0], result.size());
| ^
Member
aleks-f
added a commit
that referenced
this pull request
Dec 22, 2025
aleks-f
added a commit
that referenced
this pull request
Dec 22, 2025
aleks-f
added a commit
that referenced
this pull request
Dec 22, 2025
Fix Message initialization to correctly handle thread IDs for both POCO and non-POCO threads. When Thread::current() returns null (std::thread, main thread, etc.), use NativeThreadInfo to get the OS thread ID instead of leaving _tid as zero. Rename NativeThreadInfo::id() to NativeThreadInfo::osTid() to match the Thread::currentOsTid() naming convention and clarify that both return the same kernel-level thread identifier (not the POCO sequential ID). Additional changes: - Add comprehensive documentation to NativeThreadInfo explaining its purpose and when to use it vs Thread::current() - Fix LoggerTest warning by removing pessimizing std::move in return - Update all platform implementations (POSIX, WIN32, WINCE, VxWorks) This ensures the %I format specifier in PatternFormatter shows: - POCO thread ID (1, 2, 3...) for Poco::Thread instances - OS thread ID for non-POCO threads (std::thread, main thread, etc.)
aleks-f
added a commit
that referenced
this pull request
Dec 22, 2025
testFormatThreadName was failing in CI because it expected the POCO thread ID to be 1, but in a full test run, many threads are created before this test (by ThreadingTestSuite, etc.), resulting in higher IDs. The test now compares against the actual thread's ID (thr.id()) instead of a hard-coded value, making it robust regardless of test order.
aleks-f
added a commit
that referenced
this pull request
Dec 22, 2025
aleks-f
added a commit
that referenced
this pull request
Dec 23, 2025
aleks-f
added a commit
that referenced
this pull request
Dec 23, 2025
aleks-f
added a commit
that referenced
this pull request
Dec 23, 2025
- Thread_WIN32: Use MultiByteToWideChar with CP_UTF8 in setCurrentNameImpl instead of byte-by-byte iterator copy. The old conversion was incorrect for non-ASCII characters (e.g., Japanese thread names). - LoggerTest: Add bounds check (parts.size() >= 5) before accessing vector elements to prevent undefined behavior if log message format is unexpected.
aleks-f
added a commit
that referenced
this pull request
Dec 23, 2025
aleks-f
added a commit
that referenced
this pull request
Dec 23, 2025
Move POCO_NO_THREADNAME guard inside inline implementations instead of around declarations. This ensures the API is always available on all platforms, returning empty string or no-op on platforms without thread name support (e.g., emscripten, AIX). Update Message.cpp to remove conditional compilation since the functions are now unconditionally available.
aleks-f
added a commit
that referenced
this pull request
Dec 23, 2025
Thread_WIN32.cpp: - Use dynamic loading for SetThreadDescription/GetThreadDescription via GetProcAddress for compatibility with pre-Windows 10 1607 - setCurrentNameImpl tries modern API first, falls back to legacy exception method (0x406D1388) for older Windows/debuggers - Remove redundant processthreadsapi.h and stringapiset.h includes (already included via windows.h) LoggerTest.cpp: - Simplify loop condition (remove redundant npos check) - Fix code style (braces on new lines)
aleks-f
added a commit
that referenced
this pull request
Dec 23, 2025
* Add NativeThreadInfo + use in Message + test with PatternFormatter * Fix NativeThreadInfo for glibc without gettid * add new file to make * chore: formatting * fix(NativeThreadinfo): name and id #3333 * fix(NativeThread): add OS includes #3333 * fix(Message): use OS thread ID for non-POCO threads #3333 Fix Message initialization to correctly handle thread IDs for both POCO and non-POCO threads. When Thread::current() returns null (std::thread, main thread, etc.), use NativeThreadInfo to get the OS thread ID instead of leaving _tid as zero. Rename NativeThreadInfo::id() to NativeThreadInfo::osTid() to match the Thread::currentOsTid() naming convention and clarify that both return the same kernel-level thread identifier (not the POCO sequential ID). Additional changes: - Add comprehensive documentation to NativeThreadInfo explaining its purpose and when to use it vs Thread::current() - Fix LoggerTest warning by removing pessimizing std::move in return - Update all platform implementations (POSIX, WIN32, WINCE, VxWorks) This ensures the %I format specifier in PatternFormatter shows: - POCO thread ID (1, 2, 3...) for Poco::Thread instances - OS thread ID for non-POCO threads (std::thread, main thread, etc.) * fix(LoggerTest): compare thread ID against actual value #3333 testFormatThreadName was failing in CI because it expected the POCO thread ID to be 1, but in a full test run, many threads are created before this test (by ThreadingTestSuite, etc.), resulting in higher IDs. The test now compares against the actual thread's ID (thr.id()) instead of a hard-coded value, making it robust regardless of test order. * fix(NativeThreadInfo): windows compile #3333 * fix(NativeThreadInfo): mac compile fail; consolidate platform pthread functions selection #3333 * fix(NativeThreadInfo): use UTF8 #3333 * fix: remove NativeThreadInfo (not needed) #3333 * fix(Thread): proper UTF-8 handling and test robustness #3333 - Thread_WIN32: Use MultiByteToWideChar with CP_UTF8 in setCurrentNameImpl instead of byte-by-byte iterator copy. The old conversion was incorrect for non-ASCII characters (e.g., Japanese thread names). - LoggerTest: Add bounds check (parts.size() >= 5) before accessing vector elements to prevent undefined behavior if log message format is unexpected. * fix(Message): disable thread name on platforms that don't have it #3333 * fix(Thread): make getCurrentName/setCurrentName always available #3333 Move POCO_NO_THREADNAME guard inside inline implementations instead of around declarations. This ensures the API is always available on all platforms, returning empty string or no-op on platforms without thread name support (e.g., emscripten, AIX). Update Message.cpp to remove conditional compilation since the functions are now unconditionally available. * fix(Thread): Windows backward compatibility and code cleanup #3333 Thread_WIN32.cpp: - Use dynamic loading for SetThreadDescription/GetThreadDescription via GetProcAddress for compatibility with pre-Windows 10 1607 - setCurrentNameImpl tries modern API first, falls back to legacy exception method (0x406D1388) for older Windows/debuggers - Remove redundant processthreadsapi.h and stringapiset.h includes (already included via windows.h) LoggerTest.cpp: - Simplify loop condition (remove redundant npos check) - Fix code style (braces on new lines) --------- Co-authored-by: Olivier Smeesters <osm@idirect.net>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.