You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2013/09/11 05:51:27 UTC
git commit: Fixed the broken OsTest.process and OsTest.killtree tests.
Updated Branches:
refs/heads/master 257e3f9da -> 012d7c71c
Fixed the broken OsTest.process and OsTest.killtree tests.
Review: https://reviews.apache.org/r/14072
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/012d7c71
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/012d7c71
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/012d7c71
Branch: refs/heads/master
Commit: 012d7c71c01d7c4bf67f3f73cb23534633ce3770
Parents: 257e3f9
Author: Benjamin Mahler <bm...@twitter.com>
Authored: Fri Sep 6 16:58:43 2013 -0700
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Tue Sep 10 20:40:24 2013 -0700
----------------------------------------------------------------------
.../3rdparty/stout/include/stout/os/fork.hpp | 92 +++++++++++++-------
.../3rdparty/stout/tests/os_tests.cpp | 14 ++-
2 files changed, 72 insertions(+), 34 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/012d7c71/3rdparty/libprocess/3rdparty/stout/include/stout/os/fork.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os/fork.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os/fork.hpp
index 64e3b5d..838a5fe 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/os/fork.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os/fork.hpp
@@ -204,6 +204,26 @@ struct Fork
}
private:
+ // Represents the "tree" of descendants where each node has a
+ // pointer (into shared memory) from which we can read the
+ // descendants process information as well as a vector of children.
+ struct Tree
+ {
+ // NOTE: This struct is stored in shared memory and thus cannot
+ // hold any pointers to heap allocated memory.
+ struct Memory {
+ pid_t pid;
+ pid_t parent;
+ pid_t group;
+ pid_t session;
+
+ bool set; // Has this been initialized?
+ };
+
+ std::tr1::shared_ptr<Memory> memory;
+ std::vector<Tree> children;
+ };
+
// We use shared memory to "share" the pids of forked descendants.
// The benefit of shared memory over pipes is that each forked
// process can read its descendants' pids leading to a simpler
@@ -222,9 +242,9 @@ private:
{
SharedMemoryDeleter(int _fd) : fd(_fd) {}
- void operator () (pid_t* pid) const
+ void operator () (Tree::Memory* process) const
{
- if (munmap(pid, sizeof(pid_t)) == -1) {
+ if (munmap(process, sizeof(Tree::Memory)) == -1) {
perror("Failed to unmap memory");
abort();
}
@@ -237,15 +257,6 @@ private:
const int fd;
};
- // Represents the "tree" of descendants where each node has a
- // pointer (into shared memory) from which we can read the
- // descendants pid as well as a vector of children.
- struct Tree
- {
- std::tr1::shared_ptr<pid_t> pid;
- std::vector<Tree> children;
- };
-
// Constructs a Tree (see above) from this fork template.
Try<Tree> prepare() const
{
@@ -264,12 +275,16 @@ private:
return ErrnoError("Failed to open a shared memory object");
}
- if (ftruncate(fd, sizeof(pid_t)) == -1) {
+ if (ftruncate(fd, sizeof(Tree::Memory)) == -1) {
return ErrnoError("Failed to set size of shared memory object");
}
void* memory = mmap(
- NULL, sizeof(pid_t), PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+ NULL,
+ sizeof(Tree::Memory),
+ PROT_READ | PROT_WRITE, MAP_SHARED,
+ fd,
+ 0);
if (memory == MAP_FAILED) {
return ErrnoError("Failed to map shared memory object");
@@ -282,8 +297,9 @@ private:
SharedMemoryDeleter deleter(fd);
Tree tree;
- tree.pid = std::tr1::shared_ptr<pid_t>((pid_t*) memory, deleter);
- *tree.pid = -1;
+ tree.memory = std::tr1::shared_ptr<Tree::Memory>(
+ (Tree::Memory*) memory, deleter);
+ tree.memory->set = false;
for (size_t i = 0; i < children.size(); i++) {
Try<Tree> tree_ = children[i].prepare();
@@ -302,11 +318,19 @@ private:
{
pid_t pid = ::fork();
if (pid > 0) {
- *tree.pid = pid;
return pid;
}
- // Child process.
+ // Set the basic process information.
+ Tree::Memory process;
+ process.pid = getpid();
+ process.parent = getppid();
+ process.group = getpgid(0);
+ process.session = getsid(0);
+ process.set = true;
+
+ // Copy it into shared memory.
+ memcpy(tree.memory.get(), &process, sizeof(Tree::Memory));
// Execute the function, if any.
if (function.isSome()) {
@@ -338,27 +362,33 @@ private:
}
// Waits for all of the descendant processes in the tree to update
- // their pids and constructs a ProcessTree by calling os::process
- // for each pid.
+ // their pids and constructs a ProcessTree using the Tree::Memory
+ // information from shared memory.
static Try<ProcessTree> coordinate(const Tree& tree)
{
// Wait for the forked process.
// TODO(benh): Don't wait forever?
- while (*tree.pid == -1) {
+ while (!tree.memory->set) {
// Make sure we don't keep reading the value from a register.
__sync_synchronize();
}
- Result<Process> process = os::process(*tree.pid);
-
- if (process.isError()) {
- return Error(process.error());
- } else if (process.isNone()) {
- // TODO(benh): Use a duplicate of the current process with
- // 'tree.pid' instead, or consider copying the 'Process' struct
- // via shared memory instead of just the pid.
- return Error("Process already terminated");
- }
+ // All processes in the returned ProcessTree will have the
+ // command-line of the top level process, since we construct the
+ // tree using post-fork pre-exec information. So, we'll grab the
+ // command of the current process here.
+ Result<Process> self = os::process(getpid());
+
+ Process process = Process(
+ tree.memory->pid,
+ tree.memory->parent,
+ tree.memory->group,
+ tree.memory->session,
+ None(),
+ None(),
+ None(),
+ self.isSome() ? self.get().command : "",
+ false);
std::list<ProcessTree> children;
for (size_t i = 0; i < tree.children.size(); i++) {
@@ -369,7 +399,7 @@ private:
children.push_back(child.get());
}
- return ProcessTree(process.get(), children);
+ return ProcessTree(process, children);
}
public:
http://git-wip-us.apache.org/repos/asf/mesos/blob/012d7c71/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 0ef2eb5..71c457e 100644
--- a/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
+++ b/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
@@ -495,7 +495,7 @@ TEST_F(OsTest, pstree)
Try<ProcessTree> tree = os::pstree(getpid());
ASSERT_SOME(tree);
- EXPECT_EQ(0u, tree.get().children.size());
+ EXPECT_EQ(0u, tree.get().children.size()) << stringify(tree.get());
tree =
Fork(None(), // Child.
@@ -503,7 +503,14 @@ TEST_F(OsTest, pstree)
Exec("sleep 10"))();
ASSERT_SOME(tree);
- ASSERT_EQ(1u, tree.get().children.size());
+
+ // Depending on whether or not the shell has fork/exec'ed,
+ // we could have 1 or 2 direct children. That is, some shells
+ // might simply exec the command above (i.e., 'sleep 10') while
+ // others might fork/exec the command, keeping around a 'sh -c'
+ // process as well.
+ ASSERT_LE(1u, tree.get().children.size());
+ ASSERT_GE(2u, tree.get().children.size());
pid_t child = tree.get().process.pid;
pid_t grandchild = tree.get().children.front().process.pid;
@@ -514,7 +521,8 @@ TEST_F(OsTest, pstree)
ASSERT_SOME(tree);
EXPECT_EQ(child, tree.get().process.pid);
- ASSERT_EQ(1u, tree.get().children.size());
+ ASSERT_LE(1u, tree.get().children.size());
+ ASSERT_GE(2u, tree.get().children.size());
EXPECT_EQ(grandchild, tree.get().children.front().process.pid);
// Cleanup by killing the descendant processes.