Skip to content

Commit

Permalink
(maint) Porting Facter stable fix to Leatherman.
Browse files Browse the repository at this point in the history
Porting the fix for FACT-1128.  This fix updates the Windows execution
library to properly handle running under a job object in Windows.

The execution library will now break away from the current job if it's
possible to do so, rather than failing with a vague "access denied"
error.
  • Loading branch information
Peter Huene committed Aug 11, 2015
1 parent e09d866 commit b5fd132
Showing 1 changed file with 33 additions and 10 deletions.
43 changes: 33 additions & 10 deletions execution/src/windows/execution.cc
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,22 @@ namespace leatherman { namespace execution {
option_set<execution_options> const& options,
uint32_t timeout)
{
// Since we use a job object in the windows world, we want to
// be sure we're not in a job object, or at least able to
// break our processes out if we are in one.
BOOL in_job;
bool use_job_object = true;
if (!IsProcessInJob(GetCurrentProcess(), nullptr, &in_job)) {
throw execution_exception("could not determine if facter is running in a job object");
}
if (in_job) {
JOBOBJECT_BASIC_LIMIT_INFORMATION limits;
if (!QueryInformationJobObject(nullptr, JobObjectBasicLimitInformation, &limits, sizeof(limits), nullptr)
|| !(limits.LimitFlags & JOB_OBJECT_LIMIT_BREAKAWAY_OK)) { // short-circuits if QueryInformationJobObject fails
use_job_object = false;
}
}

// Search for the executable
string executable = which(file);
log_execution(executable.empty() ? file : executable, arguments);
Expand Down Expand Up @@ -475,7 +491,7 @@ namespace leatherman { namespace execution {
NULL, /* Don't allow child process to inherit process handle */
NULL, /* Don't allow child process to inherit thread handle */
TRUE, /* Inherit handles from the calling process for communication */
CREATE_NO_WINDOW,
use_job_object ? CREATE_NO_WINDOW | CREATE_BREAKAWAY_FROM_JOB : CREATE_NO_WINDOW,
options[execution_options::merge_environment] ? NULL : modified_environ.data(),
NULL, /* Use existing current directory */
&startupInfo, /* STARTUPINFO for child process */
Expand All @@ -495,21 +511,28 @@ namespace leatherman { namespace execution {

// Use a Job Object to group any child processes spawned by the CreateProcess invocation, so we can
// easily stop them in case of a timeout.
scoped_resource<HANDLE> hJob(CreateJobObjectW(nullptr, nullptr), CloseHandle);
if (hJob == NULL) {
LOG_ERROR("failed to create job object: %1%.", system_error());
throw execution_exception("failed to create job object.");
} else if (!AssignProcessToJobObject(hJob, hProcess)) {
LOG_ERROR("failed to associate process with job object: %1%.", system_error());
throw execution_exception("failed to associate process with job object.");
scoped_resource<HANDLE> hJob;
if (use_job_object) {
hJob = scoped_resource<HANDLE>(CreateJobObjectW(nullptr, nullptr), CloseHandle);
if (hJob == NULL) {
LOG_ERROR("failed to create job object: %1%.", system_error());
throw execution_exception("failed to create job object.");
} else if (!AssignProcessToJobObject(hJob, hProcess)) {
LOG_ERROR("failed to associate process with job object: %1%.", system_error());
throw execution_exception("failed to associate process with job object.");
}
}

bool terminate = true;
scope_exit reaper([&]() {
if (terminate) {
// Terminate the process on an exception
if (!TerminateJobObject(hJob, -1)) {
LOG_ERROR("failed to terminate process: %1%.", system_error());
if (use_job_object) {
if (!TerminateJobObject(hJob, -1)) {
LOG_ERROR("failed to terminate process: %1%.", system_error());
}
} else {
LOG_WARNING("could not terminate process %1% because a job object could not be used.", procInfo.dwProcessId);
}
}
});
Expand Down

0 comments on commit b5fd132

Please sign in to comment.