You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2015/06/25 02:28:20 UTC
[4/9] mesos git commit: Updated process::subprocess to replace
environment.
Updated process::subprocess to replace environment.
Review: https://reviews.apache.org/r/35561
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/14b49f31
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/14b49f31
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/14b49f31
Branch: refs/heads/master
Commit: 14b49f31840ff1523b31007c21b12c604700323f
Parents: 870ee5d
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sun Jun 14 11:17:32 2015 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Wed Jun 24 17:27:24 2015 -0700
----------------------------------------------------------------------
.../libprocess/include/process/subprocess.hpp | 11 ++---
3rdparty/libprocess/src/subprocess.cpp | 42 ++++++++++++++------
.../libprocess/src/tests/subprocess_tests.cpp | 7 ++--
3 files changed, 40 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/14b49f31/3rdparty/libprocess/include/process/subprocess.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/subprocess.hpp b/3rdparty/libprocess/include/process/subprocess.hpp
index 425b119..869e3d9 100644
--- a/3rdparty/libprocess/include/process/subprocess.hpp
+++ b/3rdparty/libprocess/include/process/subprocess.hpp
@@ -205,8 +205,9 @@ private:
* @param out Redirection specification for stdout.
* @param err Redirection specification for stderr.
* @param flags Flags to be stringified and appended to 'argv'.
- * @param environment Environment variables to add or overwrite
- * existing environment variables.
+ * @param environment Environment variables to use for the new
+ * subprocess or if None (the default) then the new subprocess
+ * will inherit the environment of the current process.
* @param setup Function to be invoked after forking but before
* exec'ing. NOTE: Take extra care not to invoke any
* async unsafe code in the body of this function.
@@ -214,7 +215,6 @@ private:
* subprocess.
* @return The subprocess or an error if one occured.
*/
-// TODO(dhamon): Add an option to not combine the two environments.
Try<Subprocess> subprocess(
const std::string& path,
std::vector<std::string> argv,
@@ -239,8 +239,9 @@ Try<Subprocess> subprocess(
* @param in Redirection specification for stdin.
* @param out Redirection specification for stdout.
* @param err Redirection specification for stderr.
- * @param environment Environment variables to add or overwrite
- * existing environment variables.
+ * @param environment Environment variables to use for the new
+ * subprocess or if None (the default) then the new subprocess
+ * will inherit the environment of the current process.
* @param setup Function to be invoked after forking but before
* exec'ing. NOTE: Take extra care not to invoke any
* async unsafe code in the body of this function.
http://git-wip-us.apache.org/repos/asf/mesos/blob/14b49f31/3rdparty/libprocess/src/subprocess.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/subprocess.cpp b/3rdparty/libprocess/src/subprocess.cpp
index f41f5e2..5b41f0d 100644
--- a/3rdparty/libprocess/src/subprocess.cpp
+++ b/3rdparty/libprocess/src/subprocess.cpp
@@ -20,8 +20,6 @@
#include <stout/try.hpp>
#include <stout/unreachable.hpp>
-#include <stout/os/execenv.hpp>
-
using std::map;
using std::string;
using std::vector;
@@ -110,7 +108,7 @@ static int childMain(
const Subprocess::IO& in,
const Subprocess::IO& out,
const Subprocess::IO& err,
- os::ExecEnv* envp,
+ char** envp,
const Option<lambda::function<int()>>& setup,
int stdinFd[2],
int stdoutFd[2],
@@ -160,7 +158,7 @@ static int childMain(
}
}
- os::execvpe(path.c_str(), argv, (*envp)());
+ os::execvpe(path.c_str(), argv, envp);
ABORT(string("Failed to os::execvpe in childMain: ") + strerror(errno));
}
@@ -315,19 +313,32 @@ Try<Subprocess> subprocess(
// The real arguments that will be passed to 'os::execvpe'. We need
// to construct them here before doing the clone as it might not be
- // async signal safe.
+ // async signal safe to perform the memory allocation.
char** _argv = new char*[argv.size() + 1];
for (int i = 0; i < argv.size(); i++) {
_argv[i] = (char*) argv[i].c_str();
}
_argv[argv.size()] = NULL;
- // We need to do this construction before doing the clone as it
- // might not be async-safe.
- // TODO(tillt): Consider optimizing this to not pass an empty map
- // into the constructor or even further to use execl instead of
- // execle once we have no user supplied environment.
- os::ExecEnv envp(environment.get(map<string, string>()));
+ // Like above, we need to construct the environment that we'll pass
+ // to 'os::execvpe' as it might not be async-safe to perform the
+ // memory allocations.
+ char** envp = os::environ();
+
+ if (environment.isSome()) {
+ // NOTE: We add 1 to the size for a NULL terminator.
+ envp = new char*[environment.get().size() + 1];
+
+ size_t index = 0;
+ foreachpair (const string& key, const string& value, environment.get()) {
+ string entry = key + "=" + value;
+ envp[index] = new char[entry.size() + 1];
+ strncpy(envp[index], entry.c_str(), entry.size() + 1);
+ ++index;
+ }
+
+ envp[index] = NULL;
+ }
// Determine the function to clone the child process. If the user
// does not specify the clone function, we will use the default.
@@ -342,7 +353,7 @@ Try<Subprocess> subprocess(
in,
out,
err,
- &envp,
+ envp,
setup,
stdinFd,
stdoutFd,
@@ -350,6 +361,13 @@ Try<Subprocess> subprocess(
delete[] _argv;
+ // Need to delete 'envp' if we had environment variables passed to
+ // us and we needed to allocate the space.
+ if (environment.isSome()) {
+ CHECK_NE(os::environ(), envp);
+ delete[] envp;
+ }
+
if (pid == -1) {
// Save the errno as 'close' below might overwrite it.
ErrnoError error("Failed to clone");
http://git-wip-us.apache.org/repos/asf/mesos/blob/14b49f31/3rdparty/libprocess/src/tests/subprocess_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/subprocess_tests.cpp b/3rdparty/libprocess/src/tests/subprocess_tests.cpp
index b5cfc8d..348b22d 100644
--- a/3rdparty/libprocess/src/tests/subprocess_tests.cpp
+++ b/3rdparty/libprocess/src/tests/subprocess_tests.cpp
@@ -756,13 +756,14 @@ TEST_F(SubprocessTest, EnvironmentOverride)
Clock::pause();
// Ensure we override an existing environment variable.
- os::setenv("MESSAGE", "hello");
+ os::setenv("MESSAGE1", "hello");
+ os::setenv("MESSAGE2", "world");
map<string, string> environment;
- environment["MESSAGE"] = "goodbye";
+ environment["MESSAGE2"] = "goodbye";
Try<Subprocess> s = subprocess(
- "echo $MESSAGE",
+ "echo $MESSAGE1 $MESSAGE2",
Subprocess::PIPE(),
Subprocess::PIPE(),
Subprocess::PIPE(),