Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

unify linux logging (glibc / musl) #72

Merged
merged 1 commit into from
May 1, 2016
Merged

unify linux logging (glibc / musl) #72

merged 1 commit into from
May 1, 2016

Conversation

rainbreak
Copy link
Contributor

There might be a better way of doing this. I don't know enough about musl / glibc pthreads differences to know if there is a possible common solution.

The problem is that when compiling with musl (i.e. non glibc) on linux, the changed code gives an error. Checking whether we are using glibc (rather than just linux) lets us get past this problem.

@bobsummerwill
Copy link
Contributor

"I don't know enough about musl / glibc pthreads differences to know if there is a possible common solution."

Please can you do some quick searches, and see whether this is intentionally NOT supported in musl? Or maybe some other API variant is supported, and we could switch to that?

If we know this isn't possible in musl then I'm happy to make that change, but we need to do a little due-diligence before committing this change. Thanks!

@rainbreak
Copy link
Contributor Author

http://man7.org/linux/man-pages/man7/pthreads.7.html

pthread_setname_np, pthread_getname_np - set/get the name of a thread

...
       int pthread_setname_np(pthread_t thread, const char *name);
       int pthread_getname_np(pthread_t thread, char *name, size_t len);
...

DESCRIPTION

       By default, all the threads created using pthread_create() inherit
       the program name.  

      The pthread_setname_np() function can be used to
       set a unique name for a thread, which can be useful for debugging
       multithreaded applications.  

      The thread name is a meaningful C
       language string, whose length is restricted to 16 characters,
       including the terminating null byte ('\0').  The thread argument
       specifies the thread whose name is to be changed; name specifies the
       new name.

       The pthread_getname_np() function can be used to retrieve the name of
       the thread.  The thread argument specifies the thread whose name is
       to be retrieved.  The buffer name is used to return the thread name;
       len specifies the number of bytes available in name.  The buffer
       specified by name should be at least 16 characters in length.  The
       returned thread name in the output buffer will be null terminated.

...

CONFORMING TO

       These functions are nonstandard GNU extensions.

NOTES

       pthread_setname_np() internally writes to the thread-specific comm
       file under the /proc filesystem: /proc/self/task/[tid]/comm.
       pthread_getname_np() retrieves it from the same location.

These are not currently implemented in musl as far as I can tell, e.g.

https://marc.info/?l=musl&m=146171729013062&w=1

@rainbreak
Copy link
Contributor Author

The alternative in the code is to log the thread name in a global variable, rather than attaching it to the thread itself.

ThreadLocalLogName g_logThreadName("main");

The pthread_getname_np pathway might have some debugging merit, but I don't see the advantage over g_logThreadName when we're just building.

@rainbreak
Copy link
Contributor Author

N.B. musl refuses to define __MUSL__ on principle as they are POSIX compliant and shouldn't need special casing!

@bobsummerwill
Copy link
Contributor

Good principle, except "as they are POSIX compliant" is not true in at least this case!

Want to change this PR so these two functions use a thread-local global as per your comment above? So pre-processor conditionals would stay the same but the code would change in a way which would work for GLIBC and musl.

#if defined(__GLIBC__)
pthread_setname_np(pthread_self(), _n.c_str());
#elif defined(__APPLE__)
#if defined(__APPLE__)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean like this? Or shall I just cut the pthread naming entirely for all platforms?

@rainbreak
Copy link
Contributor Author

You mean have glibc / musl do the same thread-local thing? Shall I just cut the pthread naming entirely for all platforms?

@bobsummerwill
Copy link
Contributor

Yeah - let's do this for now and keep this code for OS X. Please could you squash and update the comment and I will merge. Thanks!

Motivation is that pthread_(set|get)name_np are not implemented in musl.
This is the only non musl compliant feature across the codebase.

This commit gives consistent linux logging, regardless of glibc / musl.
@rainbreak rainbreak changed the title ignore troublesome pthread for non glibc unify linux logging (glibc / musl) May 1, 2016
@bobsummerwill bobsummerwill merged commit e6a119c into ethereum:develop May 1, 2016
@rainbreak rainbreak deleted the fix-for-musl branch May 1, 2016 10:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants