You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrei Budnik <ab...@mesosphere.com> on 2017/09/04 11:38:02 UTC

Re: Review Request 61799: Replaced ABORT with SAFE_EXIT in childhook in `subprocess_posix.hpp`.


> On Aug. 29, 2017, 3:27 p.m., James Peach wrote:
> > 3rdparty/libprocess/src/subprocess_posix.hpp
> > Line 194 (original), 195 (patched)
> > <https://reviews.apache.org/r/61799/diff/3/?file=1803750#file1803750line195>
> >
> >     Hmm. I thought about this some more and I think we should retain the `strerror` here.
> >     
> >     1. We still used it in other async-signal-safe situations.
> >     2. In glibc it only mallocs when you give it an unknown error number.
> >     3. In practice on Linux, malloc after fork is safe.
> >     4. The error message is pretty useful.
> >     
> >     Whay do you think?
> 
> Andrei Budnik wrote:
>     Ok, reverted.
> 
> Benjamin Mahler wrote:
>     > In practice on Linux, malloc after fork is safe.
>     
>     Is this true? I seem to recall seeing potential deadlocks deadlocks due to this. One example was [MESOS-7858](https://issues.apache.org/jira/browse/MESOS-7858), where some code in glibc was doing an bad assert. Sometimes when this fails, the malloc needed for assert to print would deadlock.

Taking into account previous comment from Benjamin, I've removed usage of unsafe `strerror`.


- Andrei


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61799/#review184060
-----------------------------------------------------------


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61799/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
>     https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Using ABORT causes coredump when path to an executable is invalid.
> In addition, using of SAFE_EXIT allows us to get rid of string
> concatenation, thereby making `childMain` more async-signal safe.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/subprocess_posix.hpp b478beccff22e2ffa08b9b91cfd5b9c6ada9b697 
> 
> 
> Diff: https://reviews.apache.org/r/61799/diff/5/
> 
> 
> Testing
> -------
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>