-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
#1792 Centralized the use of pthread_create and thread_setname in the create_thread_with_name function that uses thread_wrapper for compatibility across platforms and with Linux GLIBC<2.12 #1022
base: master
Are you sure you want to change the base?
Conversation
fabiodepin
commented
Apr 5, 2023
- Adjusted the thread_setname function for compatibility across platforms and with Linux GLIBC<2.12;
- Centralized the use of pthread_create and thread_setname in the create_thread_with_name function that uses thread_wrapper for compatibility;
Thanks! There was another similar PR in the queue, not sure if you saw it. I'm a little concerned about having to support systems older than 2010 though. |
…_set_name_np from pthread_np.h, solution proposed by @devnexen. memcached#952
#elif defined(__FreeBSD__) | ||
pthread_set_name_np(thread, name); | ||
#elif defined(__APPLE__) && defined(__MACH__) | ||
pthread_setname_np(name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it work as expected on macOS ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
On MACOS, the function pthread_setname_np(const char*) sets the internal name of the calling thread to the string value specified by the name argument.
Type is done with the prctl.
Now, if the rest of the source compiled under MACOS, that's another story.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. On MACOS, the function pthread_setname_np(const char*) sets the internal name of the calling thread
Sure https://github.com/apple/darwin-libpthread/blob/main/src/pthread.c#L1140 but what I meant is Linux/FreeBSD allows to specify a specific thread rather than the calling one.
Now, if the rest of the source compiled under MACOS, that's another story.
🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should build on recent MacOS: if it doesn't please report a bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure https://github.com/apple/darwin-libpthread/blob/main/src/pthread.c#L1140 but what I meant is Linux/FreeBSD allows to specify a specific thread rather than the calling one.
Yes, but as long as you have GLIBC >= 2.12. Por isso o tratamento acima usando prctl ou pthread_setname_np on Linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair point even tough glibc 2.12 is already fairly old.
I'm a little scared to ask but; what exactly are you doing that requires a glibc from before 2010, but also a modern build of memcached? I've been wanting to move the codebase to C11 (unions, initializers, atomics, etc) but things like this worry me. |
@fabiodepin can you speak to this? I'm hesitant to open the gate for supporting something so old, but I'm open to hearing about why this is necessary. I just recently acquired a cheap mac mini so I can validate this change once I've set that up. |
In my case it was needed for use on older Debian servers, which cannot be upgraded to newer versions due to other software running on the server. I can't say if more people need to use it or are using it on older systems. But for these cases, it is possible to install a second glibc on the system and use it separately with 'rpath'. I support the change to C11, I even have some optimization suggestions that I have in notes to do at another time.
|