From owner-freebsd-hackers@FreeBSD.ORG Tue Aug 14 15:15:21 2012 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4B2CA106564A; Tue, 14 Aug 2012 15:15:21 +0000 (UTC) (envelope-from listlog2011@gmail.com) Received: from mail-yw0-f54.google.com (mail-yw0-f54.google.com [209.85.213.54]) by mx1.freebsd.org (Postfix) with ESMTP id DC66E8FC12; Tue, 14 Aug 2012 15:15:20 +0000 (UTC) Received: by yhfs35 with SMTP id s35so693474yhf.13 for ; Tue, 14 Aug 2012 08:15:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:reply-to:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=qUfi/qeFMCv+7HQSCNGX57aB8TbgaWgAJpRPkmvwgQg=; b=E58xjdA3G8y7DEQL7YR4ZihDlC01+mJlu1FVy0iR7GAhS8p0FFrWijls+WSnXL8xe5 nYcTuUdbsc9z4kUGbvC+6JKiee3SuyvHSJ7BpbtdwONcphcoYK3/EHsldALfbCoZSd69 rSxC1XaxXGgi/KNEagnhccvmq80eCuRq59jxFuUBL8C5KIflndILAwmDsViFsRvDQh/t MCaFnm8A+oyQ3OAaCo4b4JL7+Yzxoim0cBUhJkvh32uwB8CjvGHG3sCotRP+WUnP4f08 QwSw4va0XtGOntK7r3996mXkw2ntYBJLy4dXdz/JsSb0C1Q/lM/C9yQMK7+NSg8wCPv9 OTJw== Received: by 10.66.78.195 with SMTP id d3mr21472239pax.17.1344957313735; Tue, 14 Aug 2012 08:15:13 -0700 (PDT) Received: from xp5k.my.domain ([220.184.79.100]) by mx.google.com with ESMTPS id nd2sm847139pbc.7.2012.08.14.08.15.09 (version=SSLv3 cipher=OTHER); Tue, 14 Aug 2012 08:15:12 -0700 (PDT) Message-ID: <502A6B7A.6070504@gmail.com> Date: Tue, 14 Aug 2012 23:15:06 +0800 From: David Xu User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:13.0) Gecko/20120808 Thunderbird/13.0.1 MIME-Version: 1.0 To: Konstantin Belousov 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> In-Reply-To: <20120814094111.GB5883@deviant.kiev.zoral.com.ua> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-hackers@freebsd.org, David Xu , Jilles Tjoelker Subject: Re: system() using vfork() or posix_spawn() and libthr X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: davidxu@freebsd.org List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Aug 2012 15:15:21 -0000 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