Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend format patterns %T and %I to support native threads #3323

Closed
osmeest opened this issue Jun 22, 2021 · 6 comments
Closed

Extend format patterns %T and %I to support native threads #3323

osmeest opened this issue Jun 22, 2021 · 6 comments

Comments

@osmeest
Copy link

osmeest commented Jun 22, 2021

PatternFormatter provides %T and %I to print the thread name and thread id. Those patterns depend on attributes of Message which are set in the init() function, using Thread::current() and calling getName() and getTid() on the thread if any.
Since Thread::current() works only for a thread that has been created using Poco::Thread, it does not work for threads that were created in another library (eg. std::thread).

@osmeest
Copy link
Author

osmeest commented Jun 22, 2021

The simplest way to add support for native threads would be to extend Message::init() and look for native thread attributes when Thread::current() returned nullptr. However, inspecting native thread attributes is going to be OS dependent, just like Thread uses multiple implementations of ThreadImpl depending on the target platform.

A cleaner way would be to split ThreadImpl in 3 parts:

  • a part that is responsible for sleeping and yielding of the current thread (it's basically a class with only static methods; it can equally well become a "Details" namespace); there is no reason for keeping those methods inside an object;
  • a part that is responsible for reading and writing attributes from a native thread (name, scheduler settings); it should also support joining threads in the native way (ie. using pthread_join but not Event::wait()); let's call it NativeThreadImpl;
    -- start would not be supported at this level
  • a part that is responsible for storing attributes before the thread is created, handling attributes that have to be set before starting (stack size), starting ; this would remain ThreadImpl but based on NativeThreadImpl.

For simplicity sake, NativeThreadImpl could provide stubbed methods for setStackSize and start, since they make no sense for already running threads. Instead of deriving from ThreadImpl, a Thread would contain a NativeThreadImpl (or a ThreadImpl when it is created by Thread constructor).

@osmeest osmeest changed the title Extended format patterns %T and %I to support native threads Extend format patterns %T and %I to support native threads Jun 23, 2021
@matejk
Copy link
Contributor

matejk commented Jun 28, 2021

There is pull request #3017 that also introduces patterns for native threads.

@osmeest
Copy link
Author

osmeest commented Jun 29, 2021

The proposals made in pull request #3017 add a new pattern for the OS specific thread id.
In the meantime, I've developed the simpler solution described above (NativeThreadInfo capable of reading id & name + platform specific implementations). It's implemented and unit tested for POSIX. I also provided blind implementations for Windows and VxWorks. Not sure how to proceed in checking those.

  • Posix (incl. MacOSX) and VxWorks have proper support for thread naming;
  • Windows has support for GetThreadDescription but only in some recent versions of Win32 API; however, it's unclear how it coexists with the "set thread description by raising an exception caught by the debugger" thing (which is used to set thread names in debugger);
  • WinCE doesn't support thread name (myimplementation returns an empty string).

Corresponding pull request #3333

@obiltschnig obiltschnig self-assigned this Jun 30, 2021
@obiltschnig obiltschnig added this to the Release 1.12.0 milestone Jun 30, 2021
@osmeest
Copy link
Author

osmeest commented Jul 6, 2021

Given that there are multiple proposals for the OS-native thread id (%O in #3017) and %I in the existing pattern, which direction do you intend to take in r1.12.0 ?

@aleks-f
Copy link
Member

aleks-f commented May 18, 2022

%J it is

@aleks-f aleks-f closed this as completed May 18, 2022
@osmeest
Copy link
Author

osmeest commented May 19, 2022

%J is ok for the OS thread id (gettid()).
But what about the thread name (pthread_getname_np()) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants