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

pthread_mutexattr_init() returns ENOSYS #1006

Closed
therealkenc opened this issue Aug 29, 2016 · 22 comments
Closed

pthread_mutexattr_init() returns ENOSYS #1006

therealkenc opened this issue Aug 29, 2016 · 22 comments

Comments

@therealkenc
Copy link
Collaborator

@therealkenc therealkenc commented Aug 29, 2016

Might as well file this one too. This first came up in #486 which (naturally) isn't getting a lot of love over on User Voice. Besides PA, the construct also rears its head in a large popular software package here. Turns out that lock_impl_posix.cc ends up in a library that is in turn used by the gn meta build system. Which, if you close your eyes, you could frame as a CLI development use case.

Build 14905 fails with: pthread_mutexattr_init: Operation not supported. I made some minor mods from the original so it compiles cleanly.

gcc -o mutex_priority mutex_priority.c -pthread

/********************************************************
 * An example source module to accompany...
 *
 * "Using POSIX Threads: Programming with Pthreads"
 *     by Brad nichols, Dick Buttlar, Jackie Farrell
 *     O'Reilly & Associates, Inc.
 *
 ********************************************************
 * mutex_priority.c
 */

#include <pthread.h>
#include <stdlib.h>
#include <string.h>
#include <sched.h>
#include <unistd.h>
#include <stdio.h>

#ifndef _POSIX_THREAD_PRIO_INHERIT
#error "This system does not support priority inherit protection in mutex"
#endif

pthread_mutex_t      m1;
pthread_mutexattr_t  mutexattr_prioinherit;

extern int
main(void)
{
  int rtn;
  int mutex_protocol;

  /* What is the default protocol on the host? */
  if ((rtn = pthread_mutexattr_init(&mutexattr_prioinherit)) != 0)
    fprintf(stderr,"pthread_mutexattr_init: %s",strerror(rtn)),exit(1);
  if ((rtn = pthread_mutexattr_getprotocol(
                       &mutexattr_prioinherit, &mutex_protocol)) != 0)
    fprintf(stderr,"pthread_mutexattr_getprotocol: %s",strerror(rtn)),exit(1);

  if (mutex_protocol == PTHREAD_PRIO_PROTECT)
    printf("Default mutex protocol is PTHREAD_PRIO_PROTECT\n");
  else if (mutex_protocol == PTHREAD_PRIO_INHERIT)
    printf("Default mutex protocol is PTHREAD_PRIO_INHERIT\n");
  else if (mutex_protocol == PTHREAD_PRIO_NONE)
    printf("Default mutex protocol is PTHREAD_PRIO_NONE\n");
  else 
    printf("Default mutex protocol is unrecognized\n");

  /* Set this mutex attribute to INHERIT */  
  if ((rtn = pthread_mutexattr_setprotocol(
                       &mutexattr_prioinherit, PTHREAD_PRIO_INHERIT)) != 0)
    fprintf(stderr,"pthread_mutexattr_setprotocol: %s\n",strerror(rtn)),exit(1);

  /* Initialize a mutex with the attribute object */
  if ((rtn = pthread_mutex_init(&m1,&mutexattr_prioinherit)) != 0)
    fprintf(stderr,"pthread_mutexattr_init: %s\n",strerror(rtn)),exit(1);

  if ((rtn = pthread_mutex_lock(&m1)) != 0)
    fprintf(stderr,"pthread_mutex_lock: %s\n",strerror(rtn)),exit(1);

  if ((rtn = pthread_mutex_unlock(&m1)) != 0)
    fprintf(stderr,"pthread_mutex_unlock: %s\n",strerror(rtn)),exit(1);

  return 0;
}
@stehufntdev
Copy link
Collaborator

@stehufntdev stehufntdev commented Aug 29, 2016

Thanks for reporting the issue. Could you please share out a strace of the failing sample so we can see what syscall pthread_mutexattr_init is using internally? Didn't see that looking through the post or the other one that was linked.

@therealkenc
Copy link
Collaborator Author

@therealkenc therealkenc commented Aug 29, 2016

Trace:

brk(0x7fffdcadf000)                     = 0x7fffdcadf000
write(1, "Default mutex protocol is PTHREA"..., 44Default mutex protocol is PTHREAD_PRIO_NONE) = 44
futex(0x7fffe3ee03ac, FUTEX_UNLOCK_PI, 0) = -1 ENOSYS (Function not implemented)
write(2, "pthread_mutexattr_init: Operatio"..., 48pthread_mutexattr_init: Operation not supported) = 48
@stehufntdev
Copy link
Collaborator

@stehufntdev stehufntdev commented Aug 29, 2016

Thanks, appreciate the help! I double checked in the code that FUTEX_UNLOCK_PI is not supported but we are tracking improvements in futex.

@marfillaster
Copy link

@marfillaster marfillaster commented Sep 1, 2016

Is this atom editor exception related ?

Assertion 'pthread_mutex_unlock(&m->mutex) == 0' failed at pulsecore/mutex-posix.c:108, function pa_mutex_unlock(). Aborting.
@therealkenc
Copy link
Collaborator Author

@therealkenc therealkenc commented Sep 2, 2016

Yes Atom/Electron is built on top of the large popular software package here.

@therealkenc
Copy link
Collaborator Author

@therealkenc therealkenc commented Sep 9, 2016

I hit this in another place today, this time Gnome GLib over here. This is in turn used by GTK+ in a number of places.

#ifdef PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP
  pthread_mutexattr_init (&attr);
  pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_ADAPTIVE_NP);
  pattr = &attr;
#endif
@therealkenc
Copy link
Collaborator Author

@therealkenc therealkenc commented Mar 8, 2017

Pinging this, and renewing my query from #996 (message) whether this can be faked for the time being. I assume mapping priority semantics to NT isn't trivial, else it would have happened already.

So, my humble request is to tell userspace they are getting priority wakeup semantics, but give them normal semantics. If an application is depending on priority semantics it might not work. Maybe. But doing this won't break anything that isn't already broken by virtue of the failed system call. Here's a typical pattern I see in straces out of (I think) glibc pthreads,

futex(0x7fffdb174a8c, FUTEX_CMP_REQUEUE_PRIVATE, 1, 2147483647, 0x7fffdb174a60, 2) = 
    -1 ENOSYS (Function not implemented)
futex(0x7fffdb174a8c, FUTEX_WAKE_PRIVATE, 2147483647) = 1

It just falls back. Much other code hard fails on the ENOSYS though.

While I am here, a quick correction on my original issue submission above, where I cited GN. That was incorrect. It looks like GN's beef is with #1353. Still, the futex ENOSYS thing must be showing up in your telemetry.

@stehufntdev
Copy link
Collaborator

@stehufntdev stehufntdev commented Mar 9, 2017

Thanks for the reminder Ken! Just to clarify the asks, it's FUTEX_UNLOCK_PI, FUTEX_CMP_REQUEUE_PRIVATE, and proper zombie support, right?

@therealkenc
Copy link
Collaborator Author

@therealkenc therealkenc commented Mar 9, 2017

Yes, those would go a long way. Technically the two in the wild are FUTEX_CMP_REQUEUE_PRIVATE and FUTEX_CMP_REQUEUE_PI_PRIVATE. Thanks!

@therealkenc
Copy link
Collaborator Author

@therealkenc therealkenc commented Mar 9, 2017

Also I should mention that I just picked the Nichols example in the OP at random to exercise the syscall. I am not sure how strong LTP is in the "priority-inheritance futexes" area. If you ever find man7 insufficient and need better test cases just ask and I'll see what I can do.

@stehufntdev stehufntdev added the bug label Apr 7, 2017
@stehufntdev
Copy link
Collaborator

@stehufntdev stehufntdev commented Apr 7, 2017

Marking this as a bug so we can track the request for FUTEX_UNLOCK_PI, FUTEX_CMP_REQUEUE_PRIVATE, and proper zombie support.

@therealkenc
Copy link
Collaborator Author

@therealkenc therealkenc commented Apr 7, 2017

@stehufntdev - If it helps prioritize post-Creators any, the futex() problem is fatal to several packages while the zombie thing is mostly in the 'annoyance' category. At great risk of looking a gift horse in the mouth, you can get more bang for the buck by (for example) spending a week going through all of /proc/ and implementing or faking entries than chasing zombies. Everything needs addressing eventually anyway, of course. If zombie support turns out fundamentally difficult for whatever reason (measured in keyboard hours), there's lots of lower hanging fruit. But whatever you get to first, it is all very appreciated.

@benhillis
Copy link
Member

@benhillis benhillis commented Aug 7, 2017

I've checked in a change that adds support for FUTEX_REQUEUE and FUTEX_CMP_REQUEUE (and the _PRIVATE versions obviously). Priority-inheritance futex operations are still on the backlog.

@therealkenc
Copy link
Collaborator Author

@therealkenc therealkenc commented Aug 8, 2017

💯. Thanks Ben, I really appreciate you taking a run at it. As you already know, #486 (unfortunately) blocks on the FUTEX_CMP_REQUEUE_PI_PRIVATE kind. But you aren't seeing that fail in Chrome now anyway, and sound doesn't have much in the way of development use cases. [Except for one, which I won't mention because it is like saying "Beetlejuice" three times.]

@Brian-Perkins
Copy link
Collaborator

@Brian-Perkins Brian-Perkins commented Mar 21, 2018

The pi-aware futex operations are now available in the Skip-ahead 17627 build.

@therealkenc
Copy link
Collaborator Author

@therealkenc therealkenc commented Mar 22, 2018

Confirmed the OP test case passes, which is more than good enough for me. Anything falling into the "priorities are not currently a supported WSL feature so there are limitations" caveat should be filed under a different cover. Thanks for squashing this.

@sneakernets
Copy link

@sneakernets sneakernets commented Mar 26, 2018

So there is a fix for this now? Thank goodness! I use WSL to do some simple audio-related operations and having sound is pretty much required for that. I've been rolling back the Fall Creators' Update for too many days now.

@yecril71pl
Copy link

@yecril71pl yecril71pl commented May 19, 2018

PulseAudio bug # 106581.

@therealkenc
Copy link
Collaborator Author

@therealkenc therealkenc commented May 19, 2018

Doing this as a PulseAudio bug is around two years too late. It will be another two years before such a "fix" finds its way into an Ubuntu LTS. If we knew FUTEX_UNLOCK_PI was going to take as long to address as it did, a work-around patch should have been upstreamed to Ubuntu's launchpad.

@tara-raj
Copy link

@tara-raj tara-raj commented Jul 16, 2018

Closing this bug since the OP ask is now fixed. Please open a new bug if you hit any additional issues. Thanks!

@trusktr
Copy link

@trusktr trusktr commented Sep 4, 2018

When will this fix make it to a non-insider version of Windows? Is it October?

@therealkenc
Copy link
Collaborator Author

@therealkenc therealkenc commented Sep 4, 2018

Yes. And yes some time in October but no guarantees in life. This goes for anything else marked as fixininsiderbuilds as well; which is one purpose of the tag.

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

Successfully merging a pull request may close this issue.

None yet
9 participants