> On June 13, 2013, 7:32 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 1118 > > <https://reviews.apache.org/r/11848/diff/1/?file=304301#file304301line1118> > > > > Is there the need to check for zombie processes? The semantics of > > "alive" would suggest that zombies are not considered alive ;) > > > > I'd be happy to add another function should we require different > > semantics at some point, but it seems at the moment we simply want an > > "alive"ness check? > > Jiang Yan Xu wrote: > Sorry I overlooked this comment. In Reaper::monitor(), if exists(pid) is > false we can directly return None() because there is nothing to monitor; if > exists(pid) is true but alive(pid) is false we can still potentially reap the > exit status. I guess this is where the semantical differences can be > leveraged. > > Jiang Yan Xu wrote: > So would you prefer a Try<bool> exits(pid) and have Try<bool> alive(pid) > return an error if the process doesn't exist (then we won't be able to > differentiate that from other errors, e.g. no permission, during liveness > check)? > > On Linux the other errors for os::alive should rarely happen. > > Vinod Kone wrote: > +1 to exists(). > > @Yan: Not sure I understand the unable to differentiate part, when the > functions returns a Try. Can you expand on your comment?
We have these states for alive(): - process alive: return true - process terminated but not reaped: return false - process nonexistent: return Error() - We don't have permission to check the status of the pid: return Error() So the bottom two are not differentiable right? Now since reaper is the only user of the functions perhaps removing them and merging the logic into reaper itself is reasonable too. What do you think? - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11848/#review21871 ----------------------------------------------------------- On June 20, 2013, 3:27 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11848/ > ----------------------------------------------------------- > > (Updated June 20, 2013, 3:27 a.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Description > ------- > > os::alive currently considers zombie processes to be alive, which is less > than optimal, as semantically they are "dead" and waiting to be reaped. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp > 1b3fb47d7567b5467fef2a2bb15d5c4a2ea42aa5 > 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/include/stout/os/process.hpp > PRE-CREATION > > Diff: https://reviews.apache.org/r/11848/diff/ > > > Testing > ------- > > This was tested through my killtree change, although it would be nice to have > tests once we have an abstraction for creating arbitrary process trees. > > > Thanks, > > Ben Mahler > >
