This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ #14782] Do not enable asynchronous cancelation in system
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: Carlos O'Donell <carlos at redhat dot com>
- Cc: Rich Felker <dalias at aerifal dot cx>, libc-alpha at sourceware dot org
- Date: Tue, 14 Jan 2014 13:43:28 +0100
- Subject: Re: [PATCH][BZ #14782] Do not enable asynchronous cancelation in system
- Authentication-results: sourceware.org; auth=none
- References: <20140112121310 dot GA28961 at domone dot podge> <20140112152839 dot GY24286 at brightrain dot aerifal dot cx> <52D2D2F7 dot 8080301 at redhat dot com>
On Sun, Jan 12, 2014 at 12:37:59PM -0500, Carlos O'Donell wrote:
> On 01/12/2014 10:28 AM, Rich Felker wrote:
> > On Sun, Jan 12, 2014 at 01:13:10PM +0100, OndÅej BÃlka wrote:
> >> Hi,
> >>
> >> When looking bugs another relatively easy one is that we do not need to
> >> enable async cancellation in system.
> >>
> >> We use cancellation to kill child process and we do not need enable
> >> cancellation until we install handlers to kill a child process. A
> >> cancellation needs to be only enabled in waidpid which already does
> >> that.
>
> This is not quite correct.
>
> The child process won't be killed via cancellation (ignore bug 14744 for
> now) because it's another process not a thread.
>
> We don't use any cancellation to kill the child. The parent is the one
> which we want to cancel.
>
> The child won't install the SIGCANCEL handler until it runs
> __pthread_initialize_minimal_internal which may be never if it's not
> MT.
>
> You are correct in that waitpid is a cancellation point and will check
> to see if this thread was the target of a cancellation request and
> cancel the thread before the function returns.
>
> Hopefully that clarifies things for you. If you have any questions about
> cancellation please ask them. It's a big mess right now in glibc, and
> I'm happy to share what I know.
>
> >> OK for 2.20?
> >>
> >> * sysdeps/posix/system.c (__libc_system): Do not enable
> >> asynchronous cancellation.
> >>
> >> diff --git a/sysdeps/posix/system.c b/sysdeps/posix/system.c
> >> index de71e6b..e8b921f 100644
> >> --- a/sysdeps/posix/system.c
> >> +++ b/sysdeps/posix/system.c
> >> @@ -181,15 +181,6 @@ __libc_system (const char *line)
> >> not be available after a chroot(), for example. */
> >> return do_system ("exit 0") == 0;
> >>
> >> - if (SINGLE_THREAD_P)
> >> - return do_system (line);
> >> -
> >> - int oldtype = LIBC_CANCEL_ASYNC ();
> >> -
> >> - int result = do_system (line);
> >> -
> >> - LIBC_CANCEL_RESET (oldtype);
> >> -
> >> - return result;
> >> + return do_system (line);
> >> }
> >> weak_alias (__libc_system, system)
>
> This looks good go me... however!
>
> OK to commit as long as you prove waitpid is actually a
> cancellation point as required by POSIX.
>
It in list here:
http://pubs.opengroup.org/onlinepubs/000095399/functions/xsh_chap02_09.html#tag_02_09_05_02