Conversation
+ use in Message + test with PatternFormatter
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.)
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.
… functions selection #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.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
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)
matejk
reviewed
Dec 23, 2025
matejk
approved these changes
Dec 23, 2025
Contributor
matejk
left a comment
There was a problem hiding this comment.
The PR is (mostly) platform-specific thread name. PR title shall reflect that.
I have just one minor comment on the PR.
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.
#3333