From: Iain Patterson Date: Thu, 23 Jan 2014 21:19:32 +0000 (+0000) Subject: Revamped environment variables again. X-Git-Tag: v2.22~39 X-Git-Url: http://git.iain.cx/?p=nssm.git;a=commitdiff_plain;h=eac4d7eedfe338703f0103aac01af2ca306f615a Revamped environment variables again. A previous commit changed how environment variables were managed. Initially we were calling SetEnvironmentVariable() on each variable defined in AppEnvironmentExtra prior to starting the service. That behaviour was changed because it was technically incorrect, potentially resulting in NSSM's own environment being harmed. Unfortunately the changed method, creating a new environment block by calling GetEnvironmentStrings() (or using the block from AppEnvironment if that and AppEnvironmentExtra were both defined), failed if users did something like set AppEnvironmentExtra to PATH=C:\bin;%PATH%. That would result in the process being started with a block which included TWO occurrences of the PATH variable; the system-defined one and the appended one. The latter would be ignored, possibly causing the application to fail to start. For this third attempt we now call GetEnvironmentStrings() once at startup, storing the block in a new variable which is only freed at exit. Immediately prior to calling CreateProcess() to start the service we use the new environment functions duplicate_environment() (on AppEnvironment) and set_environment_block() (on AppEnvironmentExtra) to set the variables. Then immediately after calling CreateProcess() we call duplicate_environment() again to restore the original block. Because they are passed to expand_environment_string(), the environment blocks are subject to expansion. That means that setting PATH as described in the example above would work. Trying to append items to the PATH by setting AppEnvironment in a similar way will probably not work. Thanks Bryan Senseman. --- diff --git a/README.txt b/README.txt index ded8b7a..85bf62e 100644 --- a/README.txt +++ b/README.txt @@ -354,7 +354,33 @@ environment variables which will be added to the service's environment. Each entry in the list should be of the form KEY=VALUE. It is possible to omit the VALUE but the = symbol is mandatory. -srvany only supports AppEnvironment. +Environment variables listed in both AppEnvironment and AppEnvironmentExtra +are subject to normal expansion, so it is possible, for example, to update the +system path by setting "PATH=C:\bin;%PATH%" in AppEnvironmentExtra. Variables +are expanded in the order in which they appear, so if you want to include the +value of one variable in another variable you should declare the dependency +first. + +Because variables defined in AppEnvironment override the existing +environment it is not possible to refer to any variables which were previously +defined. + +For example, the following AppEnvironment block: + + PATH=C:\Windows\System32;C:\Windows + PATH=C:\bin;%PATH% + +Would result in a PATH of "C:\bin;C:\Windows\System32;C:\Windows" as expected. + +Whereas the following AppEnvironment block: + + PATH=C:\bin;%PATH% + +Would result in a path containing only C:\bin and probably cause the +application to fail to start. + +Most people will want to use AppEnvironmentExtra exclusively. srvany only +supports AppEnvironment. Managing services using the GUI diff --git a/registry.cpp b/registry.cpp index 50f6dc3..bd79dc3 100644 --- a/registry.cpp +++ b/registry.cpp @@ -466,67 +466,6 @@ int get_parameters(nssm_service_t *service, STARTUPINFO *si) { /* Environment variables to add to existing rather than replace - may fail. */ get_environment(service->name, key, NSSM_REG_ENV_EXTRA, &service->env_extra, &service->env_extralen); - if (si) { - if (service->env_extra) { - TCHAR *env; - unsigned long envlen; - - /* Copy our environment for the application. */ - if (! service->env) { - TCHAR *rawenv = GetEnvironmentStrings(); - env = rawenv; - if (env) { - /* - The environment block starts with variables of the form - =C:=C:\Windows\System32 which we ignore. - */ - while (*env == _T('=')) { - for ( ; *env; env++); - env++; - } - envlen = 0; - if (*env) { - while (true) { - for ( ; env[envlen]; envlen++); - if (! env[++envlen]) break; - } - envlen++; - - service->envlen = envlen * sizeof(TCHAR); - service->env = (TCHAR *) HeapAlloc(GetProcessHeap(), 0, service->envlen); - memmove(service->env, env, service->envlen); - FreeEnvironmentStrings(rawenv); - } - } - } - - /* Append extra variables to configured variables. */ - if (service->env) { - envlen = service->envlen + service->env_extralen - sizeof(TCHAR)/*?*/; - env = (TCHAR *) HeapAlloc(GetProcessHeap(), 0, envlen); - if (env) { - memmove(env, service->env, service->envlen - sizeof(TCHAR)); - /* envlen is in bytes but env[i] is in characters. */ - memmove(env + (service->envlen / sizeof(TCHAR)) - 1, service->env_extra, service->env_extralen); - - HeapFree(GetProcessHeap(), 0, service->env); - HeapFree(GetProcessHeap(), 0, service->env_extra); - service->env = env; - service->envlen = envlen; - } - else log_event(EVENTLOG_ERROR_TYPE, NSSM_EVENT_OUT_OF_MEMORY, _T("environment"), _T("get_parameters()"), 0); - } - else { - /* Huh? No environment at all? */ - service->env = service->env_extra; - service->envlen = service->env_extralen; - } - } - - service->env_extra = 0; - service->env_extralen = 0; - } - /* Try to get priority - may fail. */ unsigned long priority; if (get_number(key, NSSM_REG_PRIORITY, &priority, false) == 1) { diff --git a/service.cpp b/service.cpp index c920af8..ee60c10 100644 --- a/service.cpp +++ b/service.cpp @@ -681,6 +681,7 @@ void cleanup_nssm_service(nssm_service_t *service) { if (service->wait_handle) UnregisterWait(service->process_handle); if (service->throttle_section_initialised) DeleteCriticalSection(&service->throttle_section); if (service->throttle_timer) CloseHandle(service->throttle_timer); + if (service->initial_env) FreeEnvironmentStrings(service->initial_env); HeapFree(GetProcessHeap(), 0, service); } @@ -1324,6 +1325,9 @@ void WINAPI service_main(unsigned long argc, TCHAR **argv) { } } + /* Remember our initial environment. */ + service->initial_env = GetEnvironmentStrings(); + monitor_service(service); } @@ -1519,23 +1523,21 @@ int start_service(nssm_service_t *service) { throttle_restart(service); + /* Set the environment. */ + if (service->env) duplicate_environment(service->env); + if (service->env_extra) set_environment_block(service->env_extra); + bool inherit_handles = false; if (si.dwFlags & STARTF_USESTDHANDLES) inherit_handles = true; unsigned long flags = service->priority & priority_mask(); if (service->stdin_pipe) flags |= DETACHED_PROCESS; if (service->affinity) flags |= CREATE_SUSPENDED; -#ifdef UNICODE - flags |= CREATE_UNICODE_ENVIRONMENT; -#endif - if (! CreateProcess(0, cmd, 0, 0, inherit_handles, flags, service->env, service->dir, &si, &pi)) { + if (! CreateProcess(0, cmd, 0, 0, inherit_handles, flags, 0, service->dir, &si, &pi)) { unsigned long exitcode = 3; unsigned long error = GetLastError(); - if (error == ERROR_INVALID_PARAMETER && service->env) { - log_event(EVENTLOG_ERROR_TYPE, NSSM_EVENT_CREATEPROCESS_FAILED_INVALID_ENVIRONMENT, service->name, service->exe, NSSM_REG_ENV, 0); - if (test_environment(service->env)) exitcode = 4; - } - else log_event(EVENTLOG_ERROR_TYPE, NSSM_EVENT_CREATEPROCESS_FAILED, service->name, service->exe, error_string(error), 0); + log_event(EVENTLOG_ERROR_TYPE, NSSM_EVENT_CREATEPROCESS_FAILED, service->name, service->exe, error_string(error), 0); close_output_handles(&si); + duplicate_environment(service->initial_env); return stop_service(service, exitcode, true, true); } service->process_handle = pi.hProcess; @@ -1545,6 +1547,9 @@ int start_service(nssm_service_t *service) { close_output_handles(&si); + /* Restore our environment. */ + duplicate_environment(service->initial_env); + if (service->affinity) { /* We are explicitly storing service->affinity as a 64-bit unsigned integer diff --git a/service.h b/service.h index 2eb2b42..fe59c9a 100644 --- a/service.h +++ b/service.h @@ -101,6 +101,7 @@ typedef struct { LARGE_INTEGER throttle_duetime; FILETIME creation_time; FILETIME exit_time; + TCHAR *initial_env; } nssm_service_t; void WINAPI service_main(unsigned long, TCHAR **);