This is an automated email from the ASF dual-hosted git repository. gilbert pushed a commit to branch 1.5.x in repository https://gitbox.apache.org/repos/asf/mesos.git
commit c75debd5756c4ba651c2672bdb40d737bc9fb76b Author: James Peach <jpe...@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 40a3884..ffb6e02 100644 --- a/src/slave/containerizer/mesos/containerizer.cpp +++ b/src/slave/containerizer/mesos/containerizer.cpp @@ -1951,6 +1951,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()); } @@ -1970,6 +1973,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"); } } @@ -1993,6 +1999,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 5fc5c30..1cdd043 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 2b93cc2..f245509 100644 --- a/src/slave/containerizer/mesos/linux_launcher.cpp +++ b/src/slave/containerizer/mesos/linux_launcher.cpp @@ -463,6 +463,15 @@ Try<pid_t> LinuxLauncherProcess::fork( child); })); + 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, @@ -489,7 +498,7 @@ Try<pid_t> LinuxLauncherProcess::fork( } }, parentHooks, - {Subprocess::ChildHook::SETSID()}); + childHooks); if (child.isError()) { return Error("Failed to clone child process: " + child.error());