Date: Tue, 14 Aug 2012 23:15:06 +0800 From: David Xu <listlog2011@gmail.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: freebsd-hackers@freebsd.org, David Xu <davidxu@freebsd.org>, Jilles Tjoelker <jilles@stack.nl> Subject: Re: system() using vfork() or posix_spawn() and libthr Message-ID: <502A6B7A.6070504@gmail.com> In-Reply-To: <20120814094111.GB5883@deviant.kiev.zoral.com.ua> References: <20120730102408.GA19983@stack.nl> <20120730105303.GU2676@deviant.kiev.zoral.com.ua> <20120805215432.GA28704@stack.nl> <20120806082535.GI2676@deviant.kiev.zoral.com.ua> <20120809105648.GA79814@stack.nl> <5029D727.2090105@freebsd.org> <20120814081830.GA5883@deviant.kiev.zoral.com.ua> <502A1788.9090702@freebsd.org> <20120814094111.GB5883@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2012/08/14 17:41, Konstantin Belousov wrote: > On Tue, Aug 14, 2012 at 05:16:56PM +0800, David Xu wrote: >> On 2012/08/14 16:18, Konstantin Belousov wrote: >>> On Tue, Aug 14, 2012 at 12:42:15PM +0800, David Xu wrote: >>>> I simply duplicated idea from OpenSolaris, here is my patch >>>> which has similar feature as your patch, and it also tries to >>>> prevent vforked child from corrupting parent's data: >>>> http://people.freebsd.org/~davidxu/patch/libthr-vfork.diff >>> You shall not return from vfork() frame in the child. Otherwise, the >>> same frame is appears to be destroyed in parent, and parent dies. More >>> often on !x86, but right combination of events on x86 is deadly too. >>> If pid or curthread local variables are spilled into stack save area, >>> then child will override them, and e.g. parent could see pid == 0, >>> returning it to caller. >>> >>> This was the reason why I went to asm wrapper for vfork. >>> >> OK. >> >>> Also, it seems that in mt process, malloc and rtld are still broken, >>> or am I missing something ? >> I will not call it as broken, malloc and rtld are working properly. >> vfork is not a normal function, for MT process, fork() is even >> very special too. >> >> POSIX says a lot about multi-threaded: >> http://pubs.opengroup.org/onlinepubs/000095399/functions/fork.html >> >> I quoted some POSIX document here: >>> A process shall be created with a single thread. If a multi-threaded >>> process calls fork(), the new process shall contain a replica of the >>> calling thread and its entire address space, possibly including the >>> states of mutexes and other resources. Consequently, to avoid errors, >>> the child process may only execute async-signal-safe operations until >>> such time as one of the exec functions is called. [THR] Fork >>> handlers may be established by means of the pthread_atfork() function >>> in order to maintain application invariants across fork() calls. >> This means child process should only do very simple things, and >> quickly call execv(). > Sure. > > But to call execv*, the child may need a working rtld, so we fixed it. > User code routinely called malloc() or even created threads in the child, > so we fixed that too. Otherwise, we have nothing to answer for the demands, > and 'other' OSes do support such usage. It is beyond POSIX, but this does > not matter, since the feature is expected to be available by application > writers. > >> For mt process, fork() is already a very complicated problem, >> one of problems I still remembered is that when fork() is called in >> signal handler, should the thread library execute pthread_atfork >> handler ? if it should do, but none of lock is async-signal safe, >> though our internal rwlock allows to be used in signal handler, >> but it is not supported by POSIX. >> Also are those atfork handler prepared to be executed in signal >> handler ? it is undefined. POSIX had opened a door here. > POSIX authors were aware of this problem. > In the rationale for SUSv4, they wrote > > "While the fork() function is async-signal-safe, there is no way for an > implementation to determine whether the fork handlers established by > pthread_atfork() are async-signal-safe. The fork handlers may attempt to > execute portions of the implementation that are not async-signal-safe, > such as those that are protected by mutexes, leading to a deadlock > condition. It is therefore undefined for the fork handlers to execute > functions that are not async-signal-safe when fork() is called from a > signal handler." > > IMO, since fork() is specified to be async-signal safe, and since fork() > is specified to call atfork() handlers, SUSv4 requires, without any > misinterpreations, that atfork calling machinery must be async-signal > safe. The only possibility for undefined behaviour is the application > code registering non-async safe handlers. But in real word, pthread atfork handlers are not async-signal safe, they mostly do mutex locking and unlocking to keep consistent state, mutex is not async-signal safe. The malloc prefork and postfork handlers happen to work because I have some hacking code in library for malloc locks. Otherwise, you even can not use fork() in signal handler. >> Above is one of complicated problem, the vfork is even more >> restrictive than fork(). >> If it is possible, I would avoid such a complicated problem >> which vfork() would cause. > I fully agree that the issues caused by vfork() in multithreaded code > are complicated, but ignoring them lowers the quality of our implementation. > Fixing vfork in multithreaded process is not trivial, but it is possible. > > My patch aims at working rtld and malloc in child. > As I said earlier, we might even try to call parent atfork handlers in child. > > Sure, if child dies at wrong time, then rtld and malloc locks and data > structures can be left in unusable state for parent, but currently we > do not work even if child is relatively well-behaving. You are requiring the thread library to implement such a mutex and other locks, that after vfork(), the mutex and other lock types must still work across processes, the PTHREAD_PROCESS_PRIVATE type of mutex and other locks now need to work in a PTHREAD_PROCESS_SHARE mode. Regards, David Xu
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?502A6B7A.6070504>