You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by id...@apache.org on 2014/05/05 22:18:25 UTC
[1/2] git commit: Add os::exists(pid_t pid) to test if pid is running.
Repository: mesos
Updated Branches:
refs/heads/master 9926af07b -> 106a63017
Add os::exists(pid_t pid) to test if pid is running.
Uses kill(0, pid) to check pid validity. This is much cheaper than using
os::process(pid) which constructs a full Process object.
Review: https://reviews.apache.org/r/20970
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/19941ab3
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/19941ab3
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/19941ab3
Branch: refs/heads/master
Commit: 19941ab314f41550467d75adf16fe6bb40995ae8
Parents: 9926af0
Author: Ian Downes <id...@twitter.com>
Authored: Fri Apr 25 18:56:53 2014 -0700
Committer: Ian Downes <id...@twitter.com>
Committed: Mon May 5 11:22:27 2014 -0700
----------------------------------------------------------------------
.../3rdparty/stout/include/stout/os/exists.hpp | 19 +++++++
.../3rdparty/stout/tests/os_tests.cpp | 57 ++++++++++++++++++++
2 files changed, 76 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/19941ab3/3rdparty/libprocess/3rdparty/stout/include/stout/os/exists.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os/exists.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os/exists.hpp
index d9860d7..9035e57 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/os/exists.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os/exists.hpp
@@ -14,6 +14,9 @@
#ifndef __STOUT_OS_EXISTS_HPP__
#define __STOUT_OS_EXISTS_HPP__
+#include <errno.h>
+#include <signal.h>
+
#include <sys/stat.h>
#include <string>
@@ -29,6 +32,22 @@ inline bool exists(const std::string& path)
return true;
}
+
+// Determine if the process identified by pid exists.
+// NOTE: Zombie processes have a pid and therefore exist. See os::process(pid)
+// to get details of a process.
+inline bool exists(pid_t pid)
+{
+ // The special signal 0 is used to check if the process exists; see kill(2).
+ // If the current user does not have permission to signal pid, but it does
+ // exist, then ::kill will return -1 and set errno == EPERM.
+ if (::kill(pid, 0) == 0 || errno == EPERM) {
+ return true;
+ }
+
+ return false;
+}
+
} // namespace os {
#endif // __STOUT_OS_EXISTS_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/19941ab3/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
index 94eb256..3e51135 100644
--- a/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
+++ b/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
@@ -650,3 +650,60 @@ TEST_F(OsTest, pstree)
// We have to reap the child for running the tests in repetition.
ASSERT_EQ(child, waitpid(child, NULL, 0));
}
+
+
+TEST_F(OsTest, ProcessExists)
+{
+ // Check we exist.
+ EXPECT_TRUE(os::exists(::getpid()));
+
+ // Check init/launchd/systemd exists.
+ // NOTE: This should return true even if we don't have permission to signal
+ // the pid.
+ EXPECT_TRUE(os::exists(1));
+
+ // Check existence of a child process through its lifecycle: running,
+ // zombied, reaped.
+ pid_t pid = ::fork();
+ ASSERT_NE(-1, pid);
+
+ if (pid == 0) {
+ // In child process.
+ while (true) { sleep(1); }
+
+ ABORT("Child should not reach this statement");
+ }
+
+ // In parent.
+ EXPECT_TRUE(os::exists(pid));
+
+ ASSERT_EQ(0, kill(pid, SIGKILL));
+
+ // Wait until the process is a zombie.
+ Duration elapsed = Duration::zero();
+ while (true) {
+ Result<os::Process> process = os::process(pid);
+ ASSERT_SOME(process);
+
+ if (process.get().zombie) {
+ break;
+ }
+
+ ASSERT_LT(elapsed, Milliseconds(100));
+
+ os::sleep(Milliseconds(5));
+ elapsed += Milliseconds(5);
+ };
+
+ // The process should still 'exist', even if it's a zombie.
+ EXPECT_TRUE(os::exists(pid));
+
+ // Reap the zombie and confirm the process no longer exists.
+ int status;
+
+ EXPECT_EQ(pid, ::waitpid(pid, &status, 0));
+ EXPECT_TRUE(WIFSIGNALED(status));
+ EXPECT_EQ(SIGKILL, WTERMSIG(status));
+
+ EXPECT_FALSE(os::exists(pid));
+}
[2/2] git commit: Update process::reap to use os::exists(pid).
Posted by id...@apache.org.
Update process::reap to use os::exists(pid).
Review: https://reviews.apache.org/r/20971
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/106a6301
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/106a6301
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/106a6301
Branch: refs/heads/master
Commit: 106a63017decd877efe4374f38653fdabbd02459
Parents: 19941ab
Author: Ian Downes <id...@twitter.com>
Authored: Fri Apr 25 19:42:09 2014 -0700
Committer: Ian Downes <id...@twitter.com>
Committed: Mon May 5 11:22:57 2014 -0700
----------------------------------------------------------------------
3rdparty/libprocess/src/reap.cpp | 47 ++++++++++++-----------------------
1 file changed, 16 insertions(+), 31 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/106a6301/3rdparty/libprocess/src/reap.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/reap.cpp b/3rdparty/libprocess/src/reap.cpp
index 274d129..b350ee1 100644
--- a/3rdparty/libprocess/src/reap.cpp
+++ b/3rdparty/libprocess/src/reap.cpp
@@ -33,19 +33,12 @@ public:
Future<Option<int> > reap(pid_t pid)
{
// Check to see if this pid exists.
- const Result<os::Process>& process = os::process(pid);
-
- if (process.isSome()) {
- // The process exists, we add it to the promises map.
+ if (os::exists(pid)) {
Owned<Promise<Option<int> > > promise(new Promise<Option<int> >());
promises.put(pid, promise);
return promise->future();
- } else if (process.isNone()) {
- return None();
} else {
- return Failure(
- "Failed to monitor process " + stringify(pid) + ": " +
- process.error());
+ return None();
}
}
@@ -54,32 +47,24 @@ protected:
void wait()
{
- // There are a few cases to consider here for each pid:
- // 1) The process is our child. In this case, we will notify
- // with the exit status once it terminates.
- // 2) The process exists but is not our child. In this case,
- // we'll notify with None() once it no longer exists, since
- // we cannot reap it.
- // 3) The process does not exist, notify with None() since it
- // has likely been reaped elsewhere.
-
+ // There are two cases to consider for each pid when it terminates:
+ // 1) The process is our child. In this case, we will reap the process and
+ // notify with the exit status.
+ // 2) The process was not our child. In this case, it will be reaped by
+ // someone else (its parent or init, if reparented) so we cannot know
+ // the exit status and we must notify with None().
+ //
+ // NOTE: A child can only be reaped by us, the parent. If a child exits
+ // between waitpid and the (!exists) conditional it will still exist as a
+ // zombie; it will be reaped by us on the next loop.
foreach (pid_t pid, promises.keys()) {
int status;
if (waitpid(pid, &status, WNOHANG) > 0) {
- // Terminated child process.
+ // We have reaped a child.
notify(pid, status);
- } else if (errno == ECHILD) {
- // The process is not our child, or does not exist. We want to
- // notify with None() once it no longer exists (reaped by
- // someone else).
- const Result<os::Process>& process = os::process(pid);
-
- if (process.isError()) {
- notify(pid, Error(process.error()));
- } else if (process.isNone()) {
- // The process has been reaped.
- notify(pid, None());
- }
+ } else if (!os::exists(pid)) {
+ // The process no longer exists and has been reaped by someone else.
+ notify(pid, None());
}
}