----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20970/#review41997 -----------------------------------------------------------
Ship it! Please document why EPERM is needed as vinod mentioned, are there permission issues? Your description suggests otherwise! :) 3rdparty/libprocess/3rdparty/stout/include/stout/os/exists.hpp <https://reviews.apache.org/r/20970/#comment75726> newline please :) 3rdparty/libprocess/3rdparty/stout/include/stout/os/exists.hpp <https://reviews.apache.org/r/20970/#comment75727> What are the semantics around zombie processes? Can you document that? 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp <https://reviews.apache.org/r/20970/#comment75729> newline please :) 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp <https://reviews.apache.org/r/20970/#comment75731> After this assertion, can you validate the fact that os::exists still returns true here? (After the kill, before the reap). If it's not returning true for zombies, then we have a problem in r/20971, so would be great to test this! 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp <https://reviews.apache.org/r/20970/#comment75730> newline here please :) - Ben Mahler On May 1, 2014, 7:17 p.m., Ian Downes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20970/ > ----------------------------------------------------------- > > (Updated May 1, 2014, 7:17 p.m.) > > > Review request for mesos, Ben Mahler and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > Uses kill(0, pid) to check pid validity. This is much cheaper than using > os::process(pid) which constructs a full Process object. Works regardless of > permission to signal the pid. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/exists.hpp > d9860d754e04f4ab5cc8f0a095b758ec62626b90 > 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp > 94eb256d24bc001660d63bd91b608988205a3d09 > > Diff: https://reviews.apache.org/r/20970/diff/ > > > Testing > ------- > > # Added a test > make check > > > Thanks, > > Ian Downes > >
