You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jp...@apache.org on 2018/08/17 22:21:44 UTC
[mesos] branch master updated (808485d -> 7569ffd)
This is an automated email from the ASF dual-hosted git repository.
jpeach pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 808485d Added MESOS-9163 to the 1.7.0 CHANGELOG.
new 2388ca4 Updated `os::pipe()` to always return O_CLOEXEC descriptors.
new 7569ffd Made the containerizer launch be explicit about O_CLOEXEC.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
3rdparty/stout/include/stout/os/posix/pipe.hpp | 52 +++++++++++++++++++++++-
src/slave/containerizer/mesos/containerizer.cpp | 9 ++++
src/slave/containerizer/mesos/launcher.cpp | 11 ++++-
src/slave/containerizer/mesos/linux_launcher.cpp | 11 ++++-
4 files changed, 80 insertions(+), 3 deletions(-)
[mesos] 02/02: Made the containerizer launch be explicit about
O_CLOEXEC.
Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jpeach pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 7569ffd05903be9b5284100f67eeffb35fcc7703
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/
---
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 98129d0..ba24180 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1991,6 +1991,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());
}
@@ -2010,6 +2013,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");
}
}
@@ -2033,6 +2039,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 ed41a14..2935bda 100644
--- a/src/slave/containerizer/mesos/launcher.cpp
+++ b/src/slave/containerizer/mesos/launcher.cpp
@@ -119,6 +119,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,
@@ -129,7 +138,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 d1f8d3f..e51352a 100644
--- a/src/slave/containerizer/mesos/linux_launcher.cpp
+++ b/src/slave/containerizer/mesos/linux_launcher.cpp
@@ -535,6 +535,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,
@@ -561,7 +570,7 @@ Try<pid_t> LinuxLauncherProcess::fork(
}
},
parentHooks,
- {Subprocess::ChildHook::SETSID()});
+ childHooks);
if (child.isError()) {
return Error("Failed to clone child process: " + child.error());
[mesos] 01/02: Updated `os::pipe()` to always return O_CLOEXEC
descriptors.
Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jpeach pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 2388ca4bd3be3ed5da266e74b518dd284de1be94
Author: James Peach <jp...@apache.org>
AuthorDate: Fri Aug 17 11:45:52 2018 -0700
Updated `os::pipe()` to always return O_CLOEXEC descriptors.
Updated `os::pipe()` to always return O_CLOEXEC descriptors,
atomically if we are on Linux or FreeBSD and the `pipe2(2)`
system call is available.
Review: https://reviews.apache.org/r/63270/
---
3rdparty/stout/include/stout/os/posix/pipe.hpp | 52 +++++++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/3rdparty/stout/include/stout/os/posix/pipe.hpp b/3rdparty/stout/include/stout/os/posix/pipe.hpp
index ac76224..9838d7b 100644
--- a/3rdparty/stout/include/stout/os/posix/pipe.hpp
+++ b/3rdparty/stout/include/stout/os/posix/pipe.hpp
@@ -15,20 +15,70 @@
#include <unistd.h>
+#include <sys/syscall.h>
+
#include <array>
#include <stout/error.hpp>
#include <stout/try.hpp>
+#include <stout/os/posix/fcntl.hpp>
+
namespace os {
-// Create pipes for interprocess communication.
+// Create pipes for interprocess communication. The pipe file descriptors
+// will be marked O_CLOEXEC (atomically if the platform supports it). To
+// pass the pipe to a child process, the caller should clear the CLOEXEC
+// flag after fork(2) but before exec(2).
inline Try<std::array<int, 2>> pipe()
{
std::array<int, 2> result;
+
+ // The pipe2() function appeared in FreeBSD 10.0.
+#if defined(_FreeBSD__) && __FreeBSD_version >= 1000000
+
+ if (::pipe2(result.data(), O_CLOEXEC) < 0) {
+ return ErrnoError();
+ }
+
+#else
+
+ // pipe2() appeared in Linux 2.6.27 and glibc 2.9.
+#if defined(__linux__) && defined(SYS_pipe2)
+ if (::syscall(SYS_pipe2, result.data(), O_CLOEXEC) == 0) {
+ return result;
+ }
+
+ // Fall back if the kernel doesn't support pipe2().
+ if (errno != ENOSYS) {
+ return ErrnoError();
+ }
+#endif
+
if (::pipe(result.data()) < 0) {
return ErrnoError();
}
+
+ Try<Nothing> cloexec = Nothing();
+
+ cloexec = os::cloexec(result[0]);
+ if (cloexec.isError()) {
+ Error error = Error("Failed to cloexec pipe: " + cloexec.error());
+ ::close(result[0]);
+ ::close(result[1]);
+ return error;
+ }
+
+ cloexec = os::cloexec(result[1]);
+ if (cloexec.isError()) {
+ Error error = Error("Failed to cloexec pipe: " + cloexec.error());
+ ::close(result[0]);
+ ::close(result[1]);
+ return error;
+ }
+
+#endif
+
return result;
}