Merge "adb: don't use setenv after forking."
diff --git a/adb/shell_service.cpp b/adb/shell_service.cpp
index 3fc70b0..0f89378 100644
--- a/adb/shell_service.cpp
+++ b/adb/shell_service.cpp
@@ -88,6 +88,9 @@
#include <termios.h>
#include <memory>
+#include <string>
+#include <unordered_map>
+#include <vector>
#include <android-base/logging.h>
#include <android-base/stringprintf.h>
@@ -237,13 +240,50 @@
ScopedFd parent_error_sfd, child_error_sfd;
char pts_name[PATH_MAX];
- // Create a socketpair for the fork() child to report any errors back to
- // the parent. Since we use threads, logging directly from the child could
- // create a race condition.
+ // Create a socketpair for the fork() child to report any errors back to the parent. Since we
+ // use threads, logging directly from the child might deadlock due to locks held in another
+ // thread during the fork.
if (!CreateSocketpair(&parent_error_sfd, &child_error_sfd)) {
LOG(ERROR) << "failed to create pipe for subprocess error reporting";
}
+ // Construct the environment for the child before we fork.
+ passwd* pw = getpwuid(getuid());
+ std::unordered_map<std::string, std::string> env;
+
+ char** current = environ;
+ while (char* env_cstr = *current++) {
+ std::string env_string = env_cstr;
+ char* delimiter = strchr(env_string.c_str(), '=');
+ *delimiter++ = '\0';
+ env[env_string.c_str()] = delimiter;
+ }
+
+ if (pw != nullptr) {
+ // TODO: $HOSTNAME? Normally bash automatically sets that, but mksh doesn't.
+ env["HOME"] = pw->pw_dir;
+ env["LOGNAME"] = pw->pw_name;
+ env["USER"] = pw->pw_name;
+ env["SHELL"] = pw->pw_shell;
+ }
+
+ if (!terminal_type_.empty()) {
+ env["TERM"] = terminal_type_;
+ }
+
+ std::vector<std::string> joined_env;
+ for (auto it : env) {
+ const char* key = it.first.c_str();
+ const char* value = it.second.c_str();
+ joined_env.push_back(android::base::StringPrintf("%s=%s", key, value));
+ }
+
+ std::vector<const char*> cenv;
+ for (const std::string& str : joined_env) {
+ cenv.push_back(str.c_str());
+ }
+ cenv.push_back(nullptr);
+
if (type_ == SubprocessType::kPty) {
int fd;
pid_ = forkpty(&fd, pts_name, nullptr, nullptr);
@@ -286,26 +326,14 @@
parent_error_sfd.Reset();
close_on_exec(child_error_sfd.fd());
- // TODO: $HOSTNAME? Normally bash automatically sets that, but mksh doesn't.
- passwd* pw = getpwuid(getuid());
- if (pw != nullptr) {
- setenv("HOME", pw->pw_dir, 1);
- setenv("LOGNAME", pw->pw_name, 1);
- setenv("SHELL", pw->pw_shell, 1);
- setenv("USER", pw->pw_name, 1);
- }
- if (!terminal_type_.empty()) {
- setenv("TERM", terminal_type_.c_str(), 1);
- }
-
if (is_interactive()) {
- execl(_PATH_BSHELL, _PATH_BSHELL, "-", nullptr);
+ execle(_PATH_BSHELL, _PATH_BSHELL, "-", nullptr, cenv.data());
} else {
- execl(_PATH_BSHELL, _PATH_BSHELL, "-c", command_.c_str(), nullptr);
+ execle(_PATH_BSHELL, _PATH_BSHELL, "-c", command_.c_str(), nullptr, cenv.data());
}
WriteFdExactly(child_error_sfd.fd(), "exec '" _PATH_BSHELL "' failed");
child_error_sfd.Reset();
- exit(-1);
+ _Exit(1);
}
// Subprocess parent.
@@ -320,6 +348,7 @@
return false;
}
+ D("subprocess parent: exec completed");
if (protocol_ == SubprocessProtocol::kNone) {
// No protocol: all streams pass through the stdinout FD and hook
// directly into the local socket for raw data transfer.
@@ -357,6 +386,7 @@
return false;
}
+ D("subprocess parent: completed");
return true;
}
