On 03/27/2011 10:21 PM, Rich Felker wrote: > On Sat, Mar 26, 2011 at 08:28:04PM +0200, Timo Teräs wrote: >> Otherwise other threads can leave malloc state locked, and the child >> will hang indefinitely if it tries to malloc something. > > This change by itself breaks as much as it fixes, unless the mutex > implementation is reentrant and recursive. fork() is specified to be > async-signal-safe, which means it's legal for a signal handler to call > fork() while malloc() is being executed. With your patch, this will > hang the program.
It is recursive mutex. It's works perfectly fine if same thread locks that mutex multiple times. Doing fork from signal handler seems utterly stupid. That would need extra precautions to work properly, because the malloc code is not re-entrant. But this is not the case I'm fixing. It's thread A doing malloc while thread B is forking. As such, this does not break anything. Or could you provide example code on scenario that this breaks (and which was not broken before this)? > I ran into the same issue developing musl's implementation of malloc, > and for now I have left it alone. POSIX 2008 (SUSv4) went back on some > of the promises of previous versions, and has now declared that it's > UB to call any async-signal-unsafe functions (such as malloc()) after > fork() in a multi-threaded program, so your patch is not needed to > conform to the current standard, only to previous versions (and even > then it's not clear that the older standard required malloc() after > fork() to work in a threaded program). Yes. I agree, it's bad practice do things like that. But it just so happens, that there are programs doing this. This can also happen indirectly in certain scenarios: e.g. using opendir/readdir of /proc/self/fd to close open fd's. (opendir in uclibc seems ot use malloc). This patch is intended to provide glibc compatibility with the more or less broken programs. > I may later revisit this issue and "fix" it, but the fix is not simply > using locks. I would either need to ignore locking and actively repair > all the potentially-corrupted malloc structures in the child process, > or selectively determine which locks are held by the current thread > (due to having interrupted malloc() with a signal handler) and only > attempt to lock the other locks before forking. For uClibc which only > has one big malloc() lock, it may be easier, but I think you'll at > least need to fix the lock to be reentrant and either recursive or > error-checking. And that of course has a cost... This prevents other threads not to execute malloc while we are forking. Thus assuring that we have consistent malloc-state for the child process. Since the thread doing fork has malloc-lock, it will own the lock, thus the lock can be safely reinitialized in the child (we need to reinitilize it since TID change on fork). When the parent continues, the lock is unlocked regularly and memory allocation can continue as normal. This exactly what glibc does and it works. This does not address the case that the thread forking is doing malloc and forks from signal handler. That case is just utterly broken and not worth fixing IMHO. - Timo _______________________________________________ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
