mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Budnik <abud...@mesosphere.com>
Subject Re: Review Request 61799: Replaced ABORT with SAFE_EXIT in childhook in `subprocess_posix.hpp`.
Date Mon, 04 Sep 2017 11:38:02 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message