Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
Fix several child process bugs on windows
Browse files Browse the repository at this point in the history
  • Loading branch information
@piscisaureus
piscisaureus committed Jan 18, 2011
1 parent 5087c62 commit bb3bf09
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 96 deletions.
2 changes: 1 addition & 1 deletion src/node_child_process.h
View file
Expand Up @@ -99,7 +99,7 @@ class ChildProcess : ObjectWrap {
static void watch(ChildProcess *child);
static void CALLBACK watch_wait_callback(void *data, BOOLEAN didTimeout);
static void notify_spawn_failure(ChildProcess *child);
static void notify_exit(ev_async *ev, int revent);
static void notify_exit(EV_P_ ev_async *ev, int revent);
static int do_kill(ChildProcess *child, int sig);static void close_stdio_handles(ChildProcess *child);

int pid_;
Expand Down
201 changes: 106 additions & 95 deletions src/node_child_process_win32.cc
View file
Expand Up @@ -50,16 +50,18 @@ static inline WCHAR* search_path_join_test(
WCHAR *result, *result_pos;

if (dir_len >= 1 && (dir[0] == L'/' || dir[0] == L'\\')) {
// It's a full path with drive letter, don't use cwd
cwd_len = 0;
} else if (dir_len == 2 && dir[1] == L':') {
// It's a full path without drive letter, use cwd's drive letter only
cwd_len = 2;
} else if (dir_len >= 2 && dir[1] == L':' &&
(dir_len < 3 || (dir[2] != L'/' && dir[2] != L'\\'))) {
// It's a relative path with drive letter (ext.g. D:../some/file)
// Replace dir by full cwd if it points to the same drive,
// Replace drive letter in dir by full cwd if it points to the same drive,
// otherwise use the dir only.
if (cwd_len < 2 || _wcsnicmp(cwd, dir, 2) != 0) {
cwd_len = 0;
} else {
dir_len = 0;
dir += 2;
dir_len -= 2;
}
} else if (dir_len > 2 && dir[1] == L':') {
// It's an absolute path with drive letter
Expand Down Expand Up @@ -327,7 +329,7 @@ void ChildProcess::close_stdio_handles(ChildProcess *child) {


// Called from the main thread
void ChildProcess::notify_exit(ev_async *ev, int revent) {
void ChildProcess::notify_exit(EV_P_ ev_async *ev, int revent) {
// Get the child process, then release the lock
ChildProcess *child = watcher_status.child;

Expand Down Expand Up @@ -355,15 +357,13 @@ void ChildProcess::notify_exit(ev_async *ev, int revent) {
}

// Close and unset the process handle
EnterCriticalSection(&child->info_lock_);
CloseHandle(child->process_handle_);
child->process_handle_ = NULL;
child->pid_ = 0;
}

LeaveCriticalSection(&child->info_lock_);

// The process never even started
child->OnExit(exit_code);
}

Expand All @@ -381,7 +381,7 @@ void ChildProcess::notify_spawn_failure(ChildProcess *child) {

watcher_status.child = child;

ev_async_send(&watcher_status.async_watcher);
ev_async_send(EV_DEFAULT_UC_ &watcher_status.async_watcher);
}


Expand All @@ -401,7 +401,7 @@ void CALLBACK ChildProcess::watch_wait_callback(void *data,
assert(result == WAIT_OBJECT_0);

watcher_status.child = child;
ev_async_send(&watcher_status.async_watcher);
ev_async_send(EV_DEFAULT_UC_ &watcher_status.async_watcher);
}


Expand All @@ -415,7 +415,7 @@ inline void ChildProcess::watch(ChildProcess *child) {
}

// We must retain the lock here because we don't want the RegisterWait
// to complete before the waithandle is set to the child process.
// to complete before the wait handle is set to the child process.
RegisterWaitForSingleObject(&child->wait_handle_, child->process_handle_,
watch_wait_callback, (void*)child, INFINITE,
WT_EXECUTEINWAITTHREAD | WT_EXECUTEONLYONCE);
Expand Down Expand Up @@ -480,54 +480,57 @@ int ChildProcess::do_spawn(eio_req *req) {
WCHAR* application_path = search_path(child->application_, child->cwd_,
child->path_, child->path_ext_);

STARTUPINFOW startup;
PROCESS_INFORMATION info;

startup.cb = sizeof(startup);
startup.lpReserved = NULL;
startup.lpDesktop = NULL;
startup.lpTitle = NULL;
startup.dwFlags = STARTF_USESTDHANDLES;
startup.cbReserved2 = 0;
startup.lpReserved2 = NULL;
startup.hStdInput = child->stdio_handles_[0];
startup.hStdOutput = child->stdio_handles_[1];
startup.hStdError = child->stdio_handles_[2];
if (application_path) {
STARTUPINFOW startup;
PROCESS_INFORMATION info;

startup.cb = sizeof(startup);
startup.lpReserved = NULL;
startup.lpDesktop = NULL;
startup.lpTitle = NULL;
startup.dwFlags = STARTF_USESTDHANDLES;
startup.cbReserved2 = 0;
startup.lpReserved2 = NULL;
startup.hStdInput = child->stdio_handles_[0];
startup.hStdOutput = child->stdio_handles_[1];
startup.hStdError = child->stdio_handles_[2];

EnterCriticalSection(&child->info_lock_);
EnterCriticalSection(&child->info_lock_);

if (!child->kill_me_) {
// Try start the process
BOOL success = CreateProcessW(
application_path,
child->arguments_,
NULL,
NULL,
1,
CREATE_UNICODE_ENVIRONMENT,
child->env_win_,
child->cwd_,
&startup,
&info
);

if (success) {
child->process_handle_ = info.hProcess;
child->pid_ = GetProcessId(info.hProcess);
child->did_start_ = true;
watch(child);

// Not interesting
CloseHandle(info.hThread);

return 0;
if (!child->kill_me_) {
// Try start the process
BOOL success = CreateProcessW(
application_path,
child->arguments_,
NULL,
NULL,
1,
CREATE_UNICODE_ENVIRONMENT,
child->env_win_,
child->cwd_,
&startup,
&info
);

if (success) {
child->process_handle_ = info.hProcess;
child->pid_ = GetProcessId(info.hProcess);
child->did_start_ = true;
watch(child);

// Not interesting
CloseHandle(info.hThread);

LeaveCriticalSection(&child->info_lock_);
return 0;
}
}

LeaveCriticalSection(&child->info_lock_);
}

// kill_me set or process failed to start
LeaveCriticalSection(&child->info_lock_);
// not found, kill_me set or process failed to start
notify_spawn_failure(child);

return 0;
}

Expand Down Expand Up @@ -560,23 +563,23 @@ int ChildProcess::after_spawn(eio_req *req) {
// Called from the main thread while eio/wait threads may still be busy with
// the process
int ChildProcess::do_kill(ChildProcess *child, int sig) {
int rv = 0;

EnterCriticalSection(&child->info_lock_);

child->exit_signal_ = sig;

if (child->did_start_) {
// On windows killed processes normally return 1
if (TerminateProcess(child->process_handle_, 1) != 0) {
return 0;
} else {
return GetLastError();
}
if (!TerminateProcess(child->process_handle_, 1))
rv = -1;
} else {
child->kill_me_ = true;
return 0;
}

LeaveCriticalSection(&child->info_lock_);

return rv;
}


Expand Down Expand Up @@ -609,57 +612,58 @@ Handle<Value> ChildProcess::Spawn(const Arguments& args) {
ChildProcess *child = ObjectWrap::Unwrap<ChildProcess>(args.Holder());

// Copy appplication name
String::Value application(args[0]->ToString());
child->application_ = _wcsdup((WCHAR*)*application);
Handle<String> app_handle = args[0]->ToString();
int app_len = app_handle->Length();
String::Value app(app_handle);
child->application_ = new WCHAR[app_len + 1];
wcsncpy(child->application_, (WCHAR*)*app, app_len + 1);

/*
* Copy second argument args[1] into a c-string called argv.
* On windows command line arguments are all quoted and concatenated to
* one string.
* Assuming that all arguments must be wrapped in quotes,
* one string. The executable name must be prepended. This is not really
* required by windows but if you don't do this programs that rely on
* argv[0] being the executable misbehave.
* Assuming that executable plus all arguments must be wrapped in quotes,
* every character needs to be quoted with a backslash,
* and every argument is followed by either a space or a nul char,
* the maximum required buffer size is Σ[arg1..argc](2 * length + 3).
* the maximum required buffer size is Σ[exe and args](2 * length + 3).
*/
Local<Array> cmd_args_handle = Local<Array>::Cast(args[1]);
int cmd_argc = cmd_args_handle->Length();

if (cmd_argc > 0) {
// Compute required buffer
int max_buf = cmd_argc * 3,
i;
for (i = 0; i < cmd_argc; i++) {
Local<String> arg_handle =
cmd_args_handle->Get(Integer::New(i))->ToString();
max_buf += 2 * arg_handle->Length();
}

child->arguments_ = new WCHAR[max_buf];
WCHAR *pos = child->arguments_;
for (i = 0; i < cmd_argc - 1; i++) {
String::Value arg(cmd_args_handle->Get(Integer::New(i))->ToString());
pos = quote_cmd_arg((WCHAR*)*arg, pos, L' ');
}
String::Value arg(cmd_args_handle->Get(Integer::New(i))->ToString());
quote_cmd_arg((WCHAR*)*arg, pos, L'\0');
// Compute required buffer
int max_buf = (1 + cmd_argc) * 3 + app_len * 2,
i;
for (i = 0; i < cmd_argc; i++) {
Local<String> arg_handle =
cmd_args_handle->Get(Integer::New(i))->ToString();
max_buf += arg_handle->Length() * 2;
}

} else {
// No arguments
child->arguments_ = _wcsdup(L"\0");
child->arguments_ = new WCHAR[max_buf];
WCHAR *pos = child->arguments_;
pos = quote_cmd_arg((WCHAR*)*app, pos, cmd_argc ? L' ' : L'\0');
for (i = 0; i < cmd_argc; i++) {
String::Value arg(cmd_args_handle->Get(Integer::New(i))->ToString());
pos = quote_cmd_arg((WCHAR*)*arg, pos, (i < cmd_argc - 1) ? L' ' : L'\0');
}

// Copy command-line arguments
// Current working directory
Local<String>cwd_handle = Local<String>::Cast(args[2]);
if (cwd_handle->Length() > 0) {
int cwd_len = cwd_handle->Length();
if (cwd_len > 0) {
// Cwd was specified
String::Value cwd(args[2]);
child->cwd_ = _wcsdup((WCHAR*)*cwd);
String::Value cwd(cwd_handle);
child->cwd_ = new WCHAR[cwd_len + 1];
wcsncpy(child->cwd_, (WCHAR*)*cwd, cwd_len + 1);
} else {
// Cwd not specified
int chars = GetCurrentDirectoryW(0, NULL);
if (!chars) {
winapi_perror("GetCurrentDirectoryW");
child->cwd_ = _wcsdup(L"");
child->cwd_ = new WCHAR[0];
child->cwd_[0] = '\0';
} else {
child->cwd_ = new WCHAR[chars];
GetCurrentDirectoryW(chars, child->cwd_);
Expand Down Expand Up @@ -765,10 +769,11 @@ Handle<Value> ChildProcess::Spawn(const Arguments& args) {
// Use this custom fd
HANDLE custom_handle = (HANDLE)_get_osfhandle(custom_fd);

// Make handle inheritable
if (!SetHandleInformation(child_handles[i], HANDLE_FLAG_INHERIT,
HANDLE_FLAG_INHERIT))
winapi_perror("SetHandleInformation");
// Make handle inheritable, don't care it it fails
// It may fail for certain types of handles - but always try to
// spawn; it'll still work for e.g. console handles
SetHandleInformation(custom_handle, HANDLE_FLAG_INHERIT,
HANDLE_FLAG_INHERIT);

has_custom_fds[i] = true;
child_handles[i] = custom_handle;
Expand All @@ -785,6 +790,9 @@ Handle<Value> ChildProcess::Spawn(const Arguments& args) {
assert(parent_fds[2] >= 0);
result->Set(2, Integer::New(parent_fds[2]));

// Grab a reference so it doesn't get GC'ed
child->Ref();

eio_custom(do_spawn, EIO_PRI_DEFAULT, after_spawn, (void*)child);

return scope.Close(result);
Expand Down Expand Up @@ -819,6 +827,9 @@ Handle<Value> ChildProcess::Kill(const Arguments& args) {
void ChildProcess::OnExit(int status) {
HandleScope scope;

// Unref() the child, as it's no longer used by threads
Unref();

handle_->Set(pid_symbol, Null());

Local<Value> onexit_v = handle_->Get(onexit_symbol);
Expand Down Expand Up @@ -859,7 +870,7 @@ void ChildProcess::Initialize(Handle<Object> target) {

target->Set(String::NewSymbol("ChildProcess"), t->GetFunction());

ev_async_init(&watcher_status.async_watcher, notify_exit);
ev_async_init(EV_DEFAULT_UC_ &watcher_status.async_watcher, notify_exit);
watcher_status.lock = CreateSemaphore(NULL, 1, 1, NULL);
}

Expand Down

0 comments on commit bb3bf09

Please sign in to comment.