You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gi...@apache.org on 2018/10/10 23:53:35 UTC
[mesos] 05/11: Made the containerizer launch be explicit about
O_CLOEXEC.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 03fe69797849335aa25e7e22b5c34f3fe622e8f5
Author: James Peach <jp...@apache.org>
AuthorDate: Fri Aug 17 11:45:56 2018 -0700
Made the containerizer launch be explicit about O_CLOEXEC.
Since the containerizer launch depends on the inherited pipe to signal
the forked child, be explicit about manipulating O_CLOEXEC on the pipe
file descriptors. Make sure to close the pipe on the error paths.
Review: https://reviews.apache.org/r/63280/
(cherry picked from commit 7569ffd05903be9b5284100f67eeffb35fcc7703)
---
src/slave/containerizer/mesos/containerizer.cpp | 9 +++++++++
src/slave/containerizer/mesos/launcher.cpp | 11 ++++++++++-
src/slave/containerizer/mesos/linux_launcher.cpp | 11 ++++++++++-
3 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 2733235..2b28e41 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1981,6 +1981,9 @@ Future<Containerizer::LaunchResult> MesosContainerizerProcess::_launch(
whitelistFds);
if (forked.isError()) {
+ os::close(pipes[0]);
+ os::close(pipes[1]);
+
return Failure("Failed to fork: " + forked.error());
}
@@ -2000,6 +2003,9 @@ Future<Containerizer::LaunchResult> MesosContainerizerProcess::_launch(
LOG(ERROR) << "Failed to checkpoint container's forked pid to '"
<< pidCheckpointPath.get() << "': " << checkpointed.error();
+ os::close(pipes[0]);
+ os::close(pipes[1]);
+
return Failure("Could not checkpoint container's pid");
}
}
@@ -2023,6 +2029,9 @@ Future<Containerizer::LaunchResult> MesosContainerizerProcess::_launch(
checkpointed = slave::state::checkpoint(pidPath, stringify(pid));
if (checkpointed.isError()) {
+ os::close(pipes[0]);
+ os::close(pipes[1]);
+
return Failure("Failed to checkpoint the container pid to"
" '" + pidPath + "': " + checkpointed.error());
}
diff --git a/src/slave/containerizer/mesos/launcher.cpp b/src/slave/containerizer/mesos/launcher.cpp
index 42aa544..845180d 100644
--- a/src/slave/containerizer/mesos/launcher.cpp
+++ b/src/slave/containerizer/mesos/launcher.cpp
@@ -120,6 +120,15 @@ Try<pid_t> SubprocessLauncher::fork(
parentHooks.emplace_back(Subprocess::ParentHook::CREATE_JOB());
#endif // __linux__
+ vector<Subprocess::ChildHook> childHooks;
+
+ childHooks.push_back(Subprocess::ChildHook::SETSID());
+
+ // TODO(jpeach) libprocess should take care of this, see MESOS-9164.
+ foreach (int_fd fd, whitelistFds) {
+ childHooks.push_back(Subprocess::ChildHook::UNSET_CLOEXEC(fd));
+ }
+
Try<Subprocess> child = subprocess(
path,
argv,
@@ -130,7 +139,7 @@ Try<pid_t> SubprocessLauncher::fork(
environment,
None(),
parentHooks,
- {Subprocess::ChildHook::SETSID()},
+ childHooks,
whitelistFds);
if (child.isError()) {
diff --git a/src/slave/containerizer/mesos/linux_launcher.cpp b/src/slave/containerizer/mesos/linux_launcher.cpp
index b176fd3..d180690 100644
--- a/src/slave/containerizer/mesos/linux_launcher.cpp
+++ b/src/slave/containerizer/mesos/linux_launcher.cpp
@@ -538,6 +538,15 @@ Try<pid_t> LinuxLauncherProcess::fork(
}));
}
+ vector<Subprocess::ChildHook> childHooks;
+
+ childHooks.push_back(Subprocess::ChildHook::SETSID());
+
+ // TODO(jpeach) libprocess should take care of this, see MESOS-9164.
+ foreach (int_fd fd, whitelistFds) {
+ childHooks.push_back(Subprocess::ChildHook::UNSET_CLOEXEC(fd));
+ }
+
Try<Subprocess> child = subprocess(
path,
argv,
@@ -564,7 +573,7 @@ Try<pid_t> LinuxLauncherProcess::fork(
}
},
parentHooks,
- {Subprocess::ChildHook::SETSID()});
+ childHooks);
if (child.isError()) {
return Error("Failed to clone child process: " + child.error());