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

add thread:setname and thread:getname #208

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

slact
Copy link

@slact slact commented Nov 3, 2018

Adds wrappers for pthread_setname_np and pthread_getname_np to the thread metatable. It's quite useful to be able to name long-running threads!

@daurnimator
Copy link
Collaborator

Is pthread_setname_np available on all cqueues supported platforms? (an _np suffix usually means "non-posix", which means that the function is not well supported)

Some quick googling suggests:

  • The API was only introduced in glibc 2.12, you may need to add a preprocessor check?
  • On FreeBSD you may need to #include <pthread_np.h>?
  • NetBSD+OpenBSD might have a different number of arguments to linux?
  • OSX has a different form again? Only lets you set for the current thread?
    This may necessitate that you copy the name away and then set it as part of thread initialisation.

src/thread.c Outdated
}
lua_pushstring(L, buf);
return 1;
} /* ct_setname() */
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't correct

@slact
Copy link
Author

slact commented Nov 6, 2018

Oh, you're right, that's quite a mess. I've added checks to explicitly support the relevant functions on Linux, the BSDs and OS X.

Copy link
Collaborator

@daurnimator daurnimator left a comment

Choose a reason for hiding this comment

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

Note that config.h.guess is from the autoguess project (https://github.com/wahern/autoguess/).
You may want to send your changes there?

config.h.guess Outdated
#endif

#ifndef HAVE_NETBSD_PTHREAD
//5.0.0 has setname() but no getname(). let's not support it for now.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use /**/ for comments

@@ -853,5 +853,33 @@
#define STRERROR_R_CHAR_P ((AG_GLIBC_PREREQ(0,0) || AG_UCLIBC_PREREQ(0,0,0)) && (HAVE__GNU_SOURCE || !(_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600)))
#endif

#ifndef HAVE_GLIBC_PTHREAD
Copy link
Collaborator

Choose a reason for hiding this comment

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

The define is only really about pthread_setname, please rename them all accordingly

Copy link
Author

Choose a reason for hiding this comment

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

Different pthread implementations have different function signatures, and some only have setname and no getname. That's why I've introduced the HAVE_<PLATFORM>_PTHREAD macro as welll as HAVE_PTHREAD_SETNAME and HAVE_PTHREAD_GETNAME.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think that really makes sense though: the values you've chosen here are very geared to the setname call.
e.g. glibc had pthreads long before 2.12.

Copy link
Author

Choose a reason for hiding this comment

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

true -- i'll use some other name then

#if HAVE_PTHREAD_SETNAME
static int ct_setname(lua_State *L) {
struct cthread *ct = ct_checkthread(L, 1);
char *name = luaL_checkstring(L, 2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

there is too much whitespace here.

src/thread.c Outdated
rc = pthread_setname_np(ct->id, "%s", name);
#elif (HAVE_FREEBSD_PTHREAD || HAVE_OPENBSD_PTHREAD)
rc = 0;
// from FreeBSD & OpenBSD man page:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use /**/ instead of //

#elif HAVE_MACOSX_PTHREAD
if (pthread_equal(ct->id, pthread_self())) {
lua_pushboolean(L, 0);
lua_pushliteral(L, "thread name cannot be set from outside the thread on this platform");

This comment was marked as resolved.

src/thread.c Outdated
case ERANGE:
lua_pushboolean(L, 0);
lua_pushliteral(L, "thread name too long");
return 2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

cqueues returns false, message, errno on failure (or just false, errno).

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

Successfully merging this pull request may close these issues.

2 participants