You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ti...@apache.org on 2014/05/29 06:03:48 UTC
git commit: Fixed ExternalContainerizer fd leak and premature close.
Repository: mesos
Updated Branches:
refs/heads/master 1af1df579 -> b0e483b86
Fixed ExternalContainerizer fd leak and premature close.
Review: https://reviews.apache.org/r/21966
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b0e483b8
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b0e483b8
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b0e483b8
Branch: refs/heads/master
Commit: b0e483b86818863fe2ae29c2489e13bbf7d76890
Parents: 1af1df5
Author: Till Toenshoff <to...@me.com>
Authored: Thu May 29 05:54:48 2014 +0200
Committer: Till Toenshoff <to...@me.com>
Committed: Thu May 29 05:54:48 2014 +0200
----------------------------------------------------------------------
.../containerizer/external_containerizer.cpp | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/b0e483b8/src/slave/containerizer/external_containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/external_containerizer.cpp b/src/slave/containerizer/external_containerizer.cpp
index 4850549..b5d0c4c 100644
--- a/src/slave/containerizer/external_containerizer.cpp
+++ b/src/slave/containerizer/external_containerizer.cpp
@@ -1144,13 +1144,16 @@ Try<Subprocess> ExternalContainerizerProcess::invoke(
// file in the executor work directory, chown'ing it if a user is
// specified. When no sandbox is given, redirect to /dev/null to
// prevent blocking on the subprocess side.
+ // TODO(tillt): Consider switching to atomic close-on-exec instead.
Try<int> err = os::open(
sandbox.isSome() ? path::join(sandbox.get().directory, "stderr")
: "/dev/null",
O_WRONLY | O_CREAT | O_APPEND | O_NONBLOCK,
S_IRUSR | S_IWUSR | S_IRGRP | S_IRWXO);
if (err.isError()) {
- return Error("Failed to redirect stderr: " + err.error());
+ return Error(
+ "Failed to redirect stderr: Failed to open: " +
+ err.error());
}
if (sandbox.isSome() && sandbox.get().user.isSome()) {
@@ -1158,12 +1161,20 @@ Try<Subprocess> ExternalContainerizerProcess::invoke(
sandbox.get().user.get(),
path::join(sandbox.get().directory, "stderr"));
if (chown.isError()) {
- return Error("Failed to redirect stderr:" + chown.error());
+ os::close(err.get());
+ return Error(
+ "Failed to redirect stderr: Failed to chown: " +
+ chown.error());
}
}
- io::splice(external.get().err(), err.get())
- .onAny(bind(&os::close, err.get()));
+ // TODO(tillt): Consider adding an overload to io::redirect
+ // that accepts a file path as 'to' for further reducing code.
+ io::redirect(external.get().err(), err.get());
+
+ // Redirect does 'dup' the file descriptor, hence we can close the
+ // original now.
+ os::close(err.get());
VLOG(2) << "Subprocess pid: " << external.get().pid() << ", "
<< "output pipe: " << external.get().out();