Date: Fri, 19 Dec 2014 22:39:18 -0500 From: Rich Felker <dalias@...c.org> To: musl@...ts.openwall.com Subject: Fixing multithreaded set*id() AS-safety Currently multithreaded set*id() functions fail to be AS-safe, despite some of them being required to be AS-safe, due to the need for locking. If they're called from a signal handler that interrupted pthread_create or dlopen (or another set*id() function during a tiny race window), attempting to obtain the lock (lock_ptc.c functions) will deadlock. There are probably other ugly issues too. I see two possible strategies for fixing this: Strategy 1: Block signals during all uses of this lock to preclude the possibility of a thread trying to take a lock it already holds. This would mean adding signal blocking to dlopen (irrelevant to performance) and pthread_create (a potentially serious cost). This feels wrong since the common uses of this lock do not require AS-safety; only the uncommon one (set*id() in a multithreaded program) does. On the other hand, pthread_create already has to manipulate the signal mask in some situations (priority attributes and when pthread_create is called from a cancelled thread), and simply making the signal masking and restoration unconditional would simplify the code at the expense of some performance cost in the common cases. I think some additional synchronization needs to be added to pthread_exit for this strategy too, since the current logic seems to have races where set*id() might deadlock due to miscounting thread when racing with pthread_exit. Strategy 2: Use the kernel, via /proc/self/task, to enumerate threads. Multithreaded set*id() would have to fail on systems with /proc not mounted or non-Linux systems that don't emulate /proc/self/task. The benefit of this approach is that we can eliminate basically all need for synchronization between important, performance-critical operations (pthread_create and pthread_exist) and the ugly set*id() mess. The only requirement seems to be ensuring that unboundedly many new threads can't be created during set*id() (imagine a chain of threads creating new threads and terminating, which reading /proc/self/task might never catch up with), but this can easily be precluded with a global flag that blocks forward progress in pthread_create (via a futex wait) if it's set, and no synchronization would be required to access the flag since it only needs to block unboundedly many new threads. There may also be some complicated logic for handling threads that exit after they're signaled but before the signal is delivered, but I think it's doable. Neither approach is really attractive. Strategy 1 feels less hackish and more elegant (it actually makes the pthread_create code more elegant than it is now by having fewer special cases), but the cost feels wasteful. Strategy 2 is ugly but has the ugliness isolated to synccall.c (the internals for set*id()) where it doesn't interact with other parts of the code in any significant way. Any opinions on which way we should go? I'll probably hold off to do any of this until the next release cycle (or maybe even later), but I want to go ahead and start thinking about and discussing it. Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.