diff options
| -rw-r--r-- | libbutl/process.cxx | 208 | ||||
| -rw-r--r-- | libbutl/utility.cxx | 52 | ||||
| -rw-r--r-- | libbutl/utility.ixx | 45 | ||||
| -rw-r--r-- | libbutl/utility.mxx | 63 | ||||
| -rw-r--r-- | tests/process/driver.cxx | 65 |
5 files changed, 316 insertions, 117 deletions
diff --git a/libbutl/process.cxx b/libbutl/process.cxx index 6c736c1..570c857 100644 --- a/libbutl/process.cxx +++ b/libbutl/process.cxx @@ -253,6 +253,35 @@ namespace butl } while (*p != nullptr); } +#if defined(LIBBUTL_POSIX_SPAWN) || defined(_WIN32) + // Return true if the NULL-terminated variable list contains an (un)set of + // the specified variable. The NULL list argument denotes an empty list. + // + // Note that on Windows variable names are case-insensitive. + // + static inline bool + contains_envvar (const char* const* vs, const char* v, size_t n) + { + if (vs != nullptr) + { + // Note that we don't expect the number of variables to (un)set to be + // large, so the linear search is OK. + // + while (const char* v1 = *vs++) + { +#ifdef _WIN32 + if (icasecmp (v1, v, n) == 0 && (v1[n] == '=' || v1[n] == '\0')) +#else + if (strncmp (v1, v, n) == 0 && (v1[n] == '=' || v1[n] == '\0')) +#endif + return true; + } + } + + return false; + } +#endif + #ifndef _WIN32 static process_path @@ -384,7 +413,7 @@ namespace butl process (const process_path& pp, const char* args[], pipe pin, pipe pout, pipe perr, const char* cwd, - const char* const* envvars) + const char* const* evars) { int in (pin.in); int out (pout.out); @@ -452,6 +481,8 @@ namespace butl else if (err == -2) in_efd.out = open_null (); + const char* const* tevars (thread_env ()); + // The posix_spawn()-based implementation. // #ifdef LIBBUTL_POSIX_SPAWN @@ -540,47 +571,45 @@ namespace butl fail (r); #endif - // Set/unset environment variables if requested. + // Set/unset the child process environment variables if requested. // - small_vector<const char*, 8> new_env; + vector<const char*> new_env; - if (envvars != nullptr) + if (tevars != nullptr || evars != nullptr) { - for (const char* const* env (environ); *env != nullptr; ++env) + // Copy the non-overridden process environment variables into the + // child's environment. + // + for (const char* const* ev (environ); *ev != nullptr; ++ev) { - // Lookup the existing variable among those that are requested to be - // (un)set. If not present, than add it to the child process - // environment. - // - // Note that on POSIX variable names are case-sensitive. - // - // Alse note that we don't expect the number of variables to (un)set - // to be large, so the linear search is OK. - // - const char* cv (*env); - const char* eq (strchr (cv, '=')); - size_t n (eq != nullptr ? eq - cv : strlen (cv)); - - const char* const* ev (envvars); - for (; *ev != nullptr; ++ev) - { - const char* v (*ev); - if (strncmp (cv, v, n) == 0 && (v[n] == '=' || v[n] == '\0')) - break; - } + const char* v (*ev); + const char* e (strchr (v, '=')); + size_t n (e != nullptr ? e - v : strlen (v)); - if (*ev == nullptr) - new_env.push_back (cv); + if (!contains_envvar (tevars, v, n) && + !contains_envvar (evars, v, n)) + new_env.push_back (v); } - // Copy the environment variables that are requested to be set. + // Copy non-overridden variable assignments into the child's + // environment. // - for (const char* const* ev (envvars); *ev != nullptr; ++ev) + auto set_vars = [&new_env] (const char* const* vs, + const char* const* ovs = nullptr) { - const char* v (*ev); - if (strchr (v, '=') != nullptr) - new_env.push_back (v); - } + if (vs != nullptr) + { + while (const char* v = *vs++) + { + const char* e (strchr (v, '=')); + if (e != nullptr && !contains_envvar (ovs, v, e - v)) + new_env.push_back (v); + } + } + }; + + set_vars (tevars, evars); + set_vars (evars); new_env.push_back (nullptr); } @@ -598,9 +627,9 @@ namespace butl &fa, nullptr /* attrp */, const_cast<char* const*> (&args[0]), - envvars != nullptr - ? const_cast<char* const*> (new_env.data ()) - : environ); + new_env.empty () + ? environ + : const_cast<char* const*> (new_env.data ())); if (r != 0) fail (r); } // Release the lock in parent. @@ -688,27 +717,33 @@ namespace butl if (cwd != nullptr && *cwd != '\0' && chdir (cwd) != 0) fail (true /* child */); - // Set/unset environment variables if requested. + // Set/unset environment variables. // - if (envvars != nullptr) + auto set_vars = [] (const char* const* vs) { - while (const char* ev = *envvars++) + if (vs != nullptr) { - const char* v (strchr (ev, '=')); - - try + while (const char* v = *vs++) { - if (v != nullptr) - setenv (string (ev, v - ev), v + 1); - else - unsetenv (ev); - } - catch (const system_error& e) - { - throw process_child_error (e.code ().value ()); + const char* e (strchr (v, '=')); + + try + { + if (e != nullptr) + setenv (string (v, e - v), e + 1); + else + unsetenv (v); + } + catch (const system_error& e) + { + throw process_child_error (e.code ().value ()); + } } } - } + }; + + set_vars (tevars); + set_vars (evars); // Try to re-exec after the "text file busy" failure for 450ms. // @@ -1334,7 +1369,7 @@ namespace butl process (const process_path& pp, const char* args[], pipe pin, pipe pout, pipe perr, const char* cwd, - const char* const* envvars) + const char* const* evars) { int in (pin.in); int out (pout.out); @@ -1356,7 +1391,9 @@ namespace butl // vector<char> new_env; - if (envvars != nullptr) + const char* const* tevars (thread_env ()); + + if (tevars != nullptr || evars != nullptr) { // The environment block contains the variables in the following format: // @@ -1365,7 +1402,7 @@ namespace butl // Note the trailing NULL character that follows the last variable // (null-terminated) string. // - unique_ptr<char, void (*)(char*)> cvars ( + unique_ptr<char, void (*)(char*)> pevars ( GetEnvironmentStringsA (), [] (char* p) { @@ -1376,50 +1413,45 @@ namespace butl assert (false); }); - if (cvars.get () == nullptr) + if (pevars.get () == nullptr) fail (); - const char* cv (cvars.get ()); - - // Copy the current environment variables. + // Copy the non-overridden process environment variables into the + // child's environment. // - while (*cv != '\0') + for (const char* v (pevars.get ()); *v != '\0'; ) { - // Lookup the existing variable among those that are requested to be - // (un)set. If not present, than copy it to the new block. - // - // Note that on Windows variable names are case-insensitive. - // - // Alse note that we don't expect the number of variables to (un)set - // to be large, so the linear search is OK. - // - size_t n (strlen (cv) + 1); // Includes NULL character. + size_t n (strlen (v) + 1); // Includes NULL character. - const char* eq (strchr (cv, '=')); - size_t nn (eq != nullptr ? eq - cv : n - 1); - const char* const* ev (envvars); - - for (; *ev != nullptr; ++ev) - { - const char* v (*ev); - if (icasecmp (cv, v, nn) == 0 && (v[nn] == '=' || v[nn] == '\0')) - break; - } + const char* e (strchr (v, '=')); + size_t nn (e != nullptr ? e - v : n - 1); - if (*ev == nullptr) - new_env.insert (new_env.end (), cv, cv + n); + if (!contains_envvar (tevars, v, nn) && + !contains_envvar (evars, v, nn)) + new_env.insert (new_env.end (), v, v + n); - cv += n; + v += n; } - // Copy the environment variables that are requested to be set. + // Copy non-overridden variable assignments into the child's + // environment. // - for (const char* const* ev (envvars); *ev != nullptr; ++ev) + auto set_vars = [&new_env] (const char* const* vs, + const char* const* ovs = nullptr) { - const char* v (*ev); - if (strchr (v, '=') != nullptr) - new_env.insert (new_env.end (), v, v + strlen (v) + 1); - } + if (vs != nullptr) + { + while (const char* v = *vs++) + { + const char* e (strchr (v, '=')); + if (e != nullptr && !contains_envvar (ovs, v, e - v)) + new_env.insert (new_env.end (), v, v + strlen (v) + 1); + } + } + }; + + set_vars (tevars, evars); + set_vars (evars); new_env.push_back ('\0'); // Terminate the new environment block. } @@ -1776,7 +1808,7 @@ namespace butl 0, // Primary thread security attributes. true, // Inherit handles. 0, // Creation flags. - envvars != nullptr ? new_env.data () : nullptr, + new_env.empty () ? nullptr : new_env.data (), cwd != nullptr && *cwd != '\0' ? cwd : nullptr, &si, &pi)) diff --git a/libbutl/utility.cxx b/libbutl/utility.cxx index bbeafd2..0a8ba1e 100644 --- a/libbutl/utility.cxx +++ b/libbutl/utility.cxx @@ -9,13 +9,14 @@ #include <libbutl/win32-utility.hxx> #endif -#include <stdlib.h> // setenv(), unsetenv(), _putenv() +#include <stdlib.h> // getenv(), setenv(), unsetenv(), _putenv() #ifndef __cpp_lib_modules_ts #include <string> #include <cstddef> #include <utility> +#include <cstring> // strncmp() #include <ostream> #include <type_traits> // enable_if, is_base_of #include <system_error> @@ -332,6 +333,55 @@ namespace butl s.resize (d - s.begin ()); } +#ifdef __cpp_thread_local + thread_local +#else + __thread +#endif + const char* const* thread_env_ = nullptr; + +#ifdef _WIN32 + const char* const* + thread_env () {return thread_env_;} + + void + thread_env (const char* const* v) {thread_env_ = v;} +#endif + + optional<std::string> + getenv (const std::string& name) + { + if (const char* const* vs = thread_env_) + { + size_t n (name.size ()); + + for (; *vs != nullptr; ++vs) + { + const char* v (*vs); + + // Note that on Windows variable names are case-insensitive. + // +#ifdef _WIN32 + if (icasecmp (name.c_str (), v, n) == 0) +#else + if (strncmp (name.c_str (), v, n) == 0) +#endif + { + switch (v[n]) + { + case '=': return string (v + n + 1); + case '\0': return nullopt; + } + } + } + } + + if (const char* r = ::getenv (name.c_str ())) + return std::string (r); + + return nullopt; + } + void setenv (const string& name, const string& value) { diff --git a/libbutl/utility.ixx b/libbutl/utility.ixx index fa37a14..40b7a84 100644 --- a/libbutl/utility.ixx +++ b/libbutl/utility.ixx @@ -4,7 +4,6 @@ #ifndef __cpp_lib_modules_ts #include <cctype> // toupper(), tolower(), is*() #include <cwctype> // isw*() -#include <cstdlib> // getenv() #include <algorithm> // for_each() #include <stdexcept> // invalid_argument #endif @@ -333,13 +332,47 @@ namespace butl return utf8_length_impl (s, nullptr, ts, wl).has_value (); } - inline optional<std::string> - getenv (const std::string& name) + // auto_thread_env + // + inline auto_thread_env:: + auto_thread_env (const char* const* new_env) { - if (const char* r = std::getenv (name.c_str ())) - return std::string (r); + // Note that this backwards logic (first initializing prev_env and then + // resetting it) is here to work around bogus "maybe used uninitialized" + // warning in GCC. + // + prev_env = thread_env (); + + if (*prev_env != new_env) + thread_env (new_env); + else + prev_env = nullopt; + } - return nullopt; + inline auto_thread_env:: + auto_thread_env (auto_thread_env&& x) + : prev_env (std::move (x.prev_env)) + { + x.prev_env = nullopt; + } + + inline auto_thread_env& auto_thread_env:: + operator= (auto_thread_env&& x) + { + if (this != &x) + { + prev_env = std::move (x.prev_env); + x.prev_env = nullopt; + } + + return *this; + } + + inline auto_thread_env:: + ~auto_thread_env () + { + if (prev_env) + thread_env (*prev_env); } template <typename F, typename P> diff --git a/libbutl/utility.mxx b/libbutl/utility.mxx index 8a0059a..f329a0f 100644 --- a/libbutl/utility.mxx +++ b/libbutl/utility.mxx @@ -266,17 +266,74 @@ LIBBUTL_MODEXPORT namespace butl // Environment variables. // - optional<std::string> + // Our getenv() wrapper (as well as the relevant process startup functions) + // have a notion of a "thread environment", that is, thread-specific + // environment variables. However, unlike the process environment (in the + // form of the environ array), the thread environment is specified as a set + // of overrides over the process environment (sets and unsets), the same as + // for the process startup. + // + extern +#ifdef __cpp_thread_local + thread_local +#else + __thread +#endif + const char* const* thread_env_; + + // On Windows one cannot export a thread-local variable so we have to + // use wrapper functions. + // +#ifdef _WIN32 + LIBBUTL_SYMEXPORT const char* const* + thread_env (); + + LIBBUTL_SYMEXPORT void + thread_env (const char* const*); +#else + inline const char* const* + thread_env () {return thread_env_;} + + inline void + thread_env (const char* const* v) {thread_env_ = v;} +#endif + + struct auto_thread_env + { + optional<const char* const*> prev_env; + + auto_thread_env () = default; + + explicit + auto_thread_env (const char* const*); + + // Move-to-empty-only type. + // + auto_thread_env (auto_thread_env&&); + auto_thread_env& operator= (auto_thread_env&&); + + auto_thread_env (const auto_thread_env&) = delete; + auto_thread_env& operator= (const auto_thread_env&) = delete; + + ~auto_thread_env (); + }; + + // Get the environment variables taking into account the current thread's + // overrides (thread_env). + // + LIBBUTL_SYMEXPORT optional<std::string> getenv (const std::string&); - // Throw system_error on failure. + // Set the process environment variable. Best done before starting any + // threads (see thread_env). Throw system_error on failure. // // Note that on Windows setting an empty value unsets the variable. // LIBBUTL_SYMEXPORT void setenv (const std::string& name, const std::string& value); - // Throw system_error on failure. + // Unset the process environment variable. Best done before starting any + // threads (see thread_env). Throw system_error on failure. // LIBBUTL_SYMEXPORT void unsetenv (const std::string&); diff --git a/tests/process/driver.cxx b/tests/process/driver.cxx index 4b5dd16..b018ac1 100644 --- a/tests/process/driver.cxx +++ b/tests/process/driver.cxx @@ -40,8 +40,6 @@ import butl.timestamp; using namespace std; using namespace butl; -static const char* envvars[] = {"ABC=1", "DEF", nullptr}; - using cstrings = vector<const char*>; bool @@ -94,18 +92,16 @@ exec (const path& p, if (bin) args.push_back ("-b"); + const char* evars[] = { + "PAR1", "PAR2=2P", "PAR6=66", "PAR7", // Override the process variables. + "THR1", "THR2=2T", // Override the thread variables. + "CHD1", // Unset a non-existing variable. + "CHD2=C2", // Add the new variable. + nullptr}; + if (env) - { args.push_back ("-e"); - // Here we set the environment variables for the current process to make - // sure that the child process will not see the variable that is requested - // to be unset, and will see the other one unaffected. - // - setenv ("DEF", "2"); - setenv ("XYZ", "3"); - } - if (cwd != nullptr) args.push_back (cwd); @@ -123,7 +119,7 @@ exec (const path& p, out ? -1 : -2, err ? (out ? 1 : -1) : -2, cwd, - env ? envvars : nullptr); + env ? evars : nullptr); try { @@ -157,12 +153,12 @@ exec (const path& p, process pr3 (args.data (), -1, -1, -2, cwd, - env ? envvars : nullptr); + env ? evars : nullptr); process pr2 (args.data (), pr, bin_mode (move (pr3.out_fd)).get (), -2, cwd, - env ? envvars : nullptr); + env ? evars : nullptr); ifdstream is (bin_mode (move (pr3.in_ofd))); o = is.read_binary (); @@ -329,12 +325,23 @@ main (int argc, const char* argv[]) if (env) { - // Check that the ABC variable is set, the DEF is unset and the XYZ is - // left unchanged. + // Check that the variables are (un)set as expected. // - if (getenv ("ABC") != optional<string> ("1") || - getenv ("DEF") || - getenv ("XYZ") != optional<string> ("3")) + if (getenv ("PAR1") || + getenv ("PAR2") != optional<string> ("2P") || + getenv ("PAR3") != optional<string> ("P3") || + getenv ("PAR4") || + getenv ("PAR5") != optional<string> ("5P") || + getenv ("PAR6") != optional<string> ("66") || + getenv ("PAR7") || + + getenv ("THR1") || + getenv ("THR2") != optional<string> ("2T") || + getenv ("THR3") != optional<string> ("T3") || + getenv ("THR4") || + + getenv ("CHD1") || + getenv ("CHD2") != optional<string> ("C2")) return 1; } @@ -364,6 +371,26 @@ main (int argc, const char* argv[]) return 0; } + // Here we set the process and thread environment variables to make sure + // that the child process will not see the variables that are requested to + // be unset, will see change for the variables that are requested to be set, + // and will see the other ones unaffected. + // + setenv ("PAR1", "P1"); + setenv ("PAR2", "P2"); + setenv ("PAR3", "P3"); + setenv ("PAR4", "P4"); + setenv ("PAR5", "P5"); + setenv ("PAR6", "P6"); + setenv ("PAR7", "P7"); + + const char* tevars[] = { + "THR1=T1", "THR2=T2", "THR3=T3", "THR4", + "PAR4", "PAR5=5P", "PAR6", "PAR7=7P", // Override the process variables. + nullptr}; + + auto_thread_env ate (tevars); + dir_path owd (dir_path::current_directory ()); // Test processes created as "already terminated". |