Skip to content

src: pass context to Get() operations for child processes #16247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cjihrig merged 3 commits into from
Oct 18, 2017
Merged

src: pass context to Get() operations for child processes #16247

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
efde153
src: increase usage of context in process_wrap.cc
cjihrig Oct 17, 2017
ad07065
src: increase usage of context in spawn_sync.cc
cjihrig Oct 17, 2017
6dcc37d
src: combine loops in CopyJsStringArray()
cjihrig Oct 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
src: increase usage of context in spawn_sync.cc 
Refs: #15380
PR-URL: #16247
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@cjihrig
cjihrig committed Oct 18, 2017
commit ad0706551204667107d72b99e6cfc15bb95844fc
131 changes: 84 additions & 47 deletions src/spawn_sync.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,39 +663,47 @@ void SyncProcessRunner::SetPipeError(int pipe_error) {

Local<Object> SyncProcessRunner::BuildResultObject() {
EscapableHandleScope scope(env()->isolate());
Local<Context> context = env()->context();

Local<Object> js_result = Object::New(env()->isolate());

if (GetError() != 0) {
js_result->Set(env()->error_string(),
Integer::New(env()->isolate(), GetError()));
js_result->Set(context, env()->error_string(),
Integer::New(env()->isolate(), GetError())).FromJust();
}

if (exit_status_ >= 0) {
if (term_signal_ > 0) {
js_result->Set(env()->status_string(), Null(env()->isolate()));
js_result->Set(context, env()->status_string(),
Null(env()->isolate())).FromJust();
} else {
js_result->Set(env()->status_string(),
Number::New(env()->isolate(), static_cast<double>(exit_status_)));
js_result->Set(context, env()->status_string(),
Number::New(env()->isolate(),
static_cast<double>(exit_status_))).FromJust();
}
} else {
// If exit_status_ < 0 the process was never started because of some error.
js_result->Set(env()->status_string(), Null(env()->isolate()));
js_result->Set(context, env()->status_string(),
Null(env()->isolate())).FromJust();
}

if (term_signal_ > 0)
js_result->Set(env()->signal_string(),
String::NewFromUtf8(env()->isolate(), signo_string(term_signal_)));
js_result->Set(context, env()->signal_string(),
String::NewFromUtf8(env()->isolate(),
signo_string(term_signal_))).FromJust();
else
js_result->Set(env()->signal_string(), Null(env()->isolate()));
js_result->Set(context, env()->signal_string(),
Null(env()->isolate())).FromJust();

if (exit_status_ >= 0)
js_result->Set(env()->output_string(), BuildOutputArray());
js_result->Set(context, env()->output_string(),
BuildOutputArray()).FromJust();
else
js_result->Set(env()->output_string(), Null(env()->isolate()));
js_result->Set(context, env()->output_string(),
Null(env()->isolate())).FromJust();

js_result->Set(env()->pid_string(),
Number::New(env()->isolate(), uv_process_.pid));
js_result->Set(context, env()->pid_string(),
Number::New(env()->isolate(), uv_process_.pid)).FromJust();

return scope.Escape(js_result);
}
Expand All @@ -706,14 +714,15 @@ Local<Array> SyncProcessRunner::BuildOutputArray() {
CHECK_NE(stdio_pipes_, nullptr);

EscapableHandleScope scope(env()->isolate());
Local<Context> context = env()->context();
Local<Array> js_output = Array::New(env()->isolate(), stdio_count_);

for (uint32_t i = 0; i < stdio_count_; i++) {
SyncProcessStdioPipe* h = stdio_pipes_[i];
if (h != nullptr && h->writable())
js_output->Set(i, h->GetOutputAsBuffer(env()));
js_output->Set(context, i, h->GetOutputAsBuffer(env())).FromJust();
else
js_output->Set(i, Null(env()->isolate()));
js_output->Set(context, i, Null(env()->isolate())).FromJust();
}

return scope.Escape(js_output);
Expand All @@ -727,86 +736,100 @@ int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
if (!js_value->IsObject())
return UV_EINVAL;

Local<Context> context = env()->context();
Local<Object> js_options = js_value.As<Object>();

Local<Value> js_file = js_options->Get(env()->file_string());
Local<Value> js_file =
js_options->Get(context, env()->file_string()).ToLocalChecked();
r = CopyJsString(js_file, &file_buffer_);
if (r < 0)
return r;
uv_process_options_.file = file_buffer_;

Local<Value> js_args = js_options->Get(env()->args_string());
Local<Value> js_args =
js_options->Get(context, env()->args_string()).ToLocalChecked();
r = CopyJsStringArray(js_args, &args_buffer_);
if (r < 0)
return r;
uv_process_options_.args = reinterpret_cast<char**>(args_buffer_);


Local<Value> js_cwd = js_options->Get(env()->cwd_string());
Local<Value> js_cwd =
js_options->Get(context, env()->cwd_string()).ToLocalChecked();
if (IsSet(js_cwd)) {
r = CopyJsString(js_cwd, &cwd_buffer_);
if (r < 0)
return r;
uv_process_options_.cwd = cwd_buffer_;
}

Local<Value> js_env_pairs = js_options->Get(env()->env_pairs_string());
Local<Value> js_env_pairs =
js_options->Get(context, env()->env_pairs_string()).ToLocalChecked();
if (IsSet(js_env_pairs)) {
r = CopyJsStringArray(js_env_pairs, &env_buffer_);
if (r < 0)
return r;

uv_process_options_.env = reinterpret_cast<char**>(env_buffer_);
}
Local<Value> js_uid = js_options->Get(env()->uid_string());
Local<Value> js_uid =
js_options->Get(context, env()->uid_string()).ToLocalChecked();
if (IsSet(js_uid)) {
CHECK(js_uid->IsInt32());
const int32_t uid = js_uid->Int32Value(env()->context()).FromJust();
const int32_t uid = js_uid->Int32Value(context).FromJust();
uv_process_options_.uid = static_cast<uv_uid_t>(uid);
uv_process_options_.flags |= UV_PROCESS_SETUID;
}

Local<Value> js_gid = js_options->Get(env()->gid_string());
Local<Value> js_gid =
js_options->Get(context, env()->gid_string()).ToLocalChecked();
if (IsSet(js_gid)) {
CHECK(js_gid->IsInt32());
const int32_t gid = js_gid->Int32Value(env()->context()).FromJust();
const int32_t gid = js_gid->Int32Value(context).FromJust();
uv_process_options_.gid = static_cast<uv_gid_t>(gid);
uv_process_options_.flags |= UV_PROCESS_SETGID;
}

if (js_options->Get(env()->detached_string())->BooleanValue())
Local<Value> js_detached =
js_options->Get(context, env()->detached_string()).ToLocalChecked();
if (js_detached->BooleanValue(context).FromJust())
uv_process_options_.flags |= UV_PROCESS_DETACHED;

Local<String> win_hide = env()->windows_hide_string();

if (js_options->Get(win_hide)->BooleanValue())
Local<Value> js_win_hide =
js_options->Get(context, env()->windows_hide_string()).ToLocalChecked();
if (js_win_hide->BooleanValue(context).FromJust())
uv_process_options_.flags |= UV_PROCESS_WINDOWS_HIDE;

Local<String> wba = env()->windows_verbatim_arguments_string();
Local<Value> js_wva =
js_options->Get(context, env()->windows_verbatim_arguments_string())
.ToLocalChecked();

if (js_options->Get(wba)->BooleanValue())
if (js_wva->BooleanValue(context).FromJust())
uv_process_options_.flags |= UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS;

Local<Value> js_timeout = js_options->Get(env()->timeout_string());
Local<Value> js_timeout =
js_options->Get(context, env()->timeout_string()).ToLocalChecked();
if (IsSet(js_timeout)) {
CHECK(js_timeout->IsNumber());
int64_t timeout = js_timeout->IntegerValue();
int64_t timeout = js_timeout->IntegerValue(context).FromJust();
timeout_ = static_cast<uint64_t>(timeout);
}

Local<Value> js_max_buffer = js_options->Get(env()->max_buffer_string());
Local<Value> js_max_buffer =
js_options->Get(context, env()->max_buffer_string()).ToLocalChecked();
if (IsSet(js_max_buffer)) {
CHECK(js_max_buffer->IsNumber());
max_buffer_ = js_max_buffer->NumberValue();
max_buffer_ = js_max_buffer->NumberValue(context).FromJust();
}

Local<Value> js_kill_signal = js_options->Get(env()->kill_signal_string());
Local<Value> js_kill_signal =
js_options->Get(context, env()->kill_signal_string()).ToLocalChecked();
if (IsSet(js_kill_signal)) {
CHECK(js_kill_signal->IsInt32());
kill_signal_ = js_kill_signal->Int32Value();
kill_signal_ = js_kill_signal->Int32Value(context).FromJust();
}

Local<Value> js_stdio = js_options->Get(env()->stdio_string());
Local<Value> js_stdio =
js_options->Get(context, env()->stdio_string()).ToLocalChecked();
r = ParseStdioOptions(js_stdio);
if (r < 0)
return r;
Expand All @@ -822,6 +845,7 @@ int SyncProcessRunner::ParseStdioOptions(Local<Value> js_value) {
if (!js_value->IsArray())
return UV_EINVAL;

Local<Context> context = env()->context();
js_stdio_options = js_value.As<Array>();

stdio_count_ = js_stdio_options->Length();
Expand All @@ -831,7 +855,8 @@ int SyncProcessRunner::ParseStdioOptions(Local<Value> js_value) {
stdio_pipes_initialized_ = true;

for (uint32_t i = 0; i < stdio_count_; i++) {
Local<Value> js_stdio_option = js_stdio_options->Get(i);
Local<Value> js_stdio_option =
js_stdio_options->Get(context, i).ToLocalChecked();

if (!js_stdio_option->IsObject())
return UV_EINVAL;
Expand All @@ -850,7 +875,9 @@ int SyncProcessRunner::ParseStdioOptions(Local<Value> js_value) {

int SyncProcessRunner::ParseStdioOption(int child_fd,
Local<Object> js_stdio_option) {
Local<Value> js_type = js_stdio_option->Get(env()->type_string());
Local<Context> context = env()->context();
Local<Value> js_type =
js_stdio_option->Get(context, env()->type_string()).ToLocalChecked();

if (js_type->StrictEquals(env()->ignore_string())) {
return AddStdioIgnore(child_fd);
Expand All @@ -859,13 +886,17 @@ int SyncProcessRunner::ParseStdioOption(int child_fd,
Local<String> rs = env()->readable_string();
Local<String> ws = env()->writable_string();

bool readable = js_stdio_option->Get(rs)->BooleanValue();
bool writable = js_stdio_option->Get(ws)->BooleanValue();
bool readable = js_stdio_option->Get(context, rs)
.ToLocalChecked()->BooleanValue(context).FromJust();
bool writable =
js_stdio_option->Get(context, ws)
.ToLocalChecked()->BooleanValue(context).FromJust();

uv_buf_t buf = uv_buf_init(nullptr, 0);

if (readable) {
Local<Value> input = js_stdio_option->Get(env()->input_string());
Local<Value> input =
js_stdio_option->Get(context, env()->input_string()).ToLocalChecked();
if (Buffer::HasInstance(input)) {
buf = uv_buf_init(Buffer::Data(input),
static_cast<unsigned int>(Buffer::Length(input)));
Expand All @@ -881,7 +912,8 @@ int SyncProcessRunner::ParseStdioOption(int child_fd,

} else if (js_type->StrictEquals(env()->inherit_string()) ||
js_type->StrictEquals(env()->fd_string())) {
int inherit_fd = js_stdio_option->Get(env()->fd_string())->Int32Value();
int inherit_fd = js_stdio_option->Get(context, env()->fd_string())
.ToLocalChecked()->Int32Value(context).FromJust();
return AddStdioInheritFD(child_fd, inherit_fd);

} else {
Expand Down Expand Up @@ -981,14 +1013,17 @@ int SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
if (!js_value->IsArray())
return UV_EINVAL;

Local<Context> context = env()->context();
js_array = js_value.As<Array>()->Clone().As<Array>();
length = js_array->Length();

// Convert all array elements to string. Modify the js object itself if
// needed - it's okay since we cloned the original object.
for (uint32_t i = 0; i < length; i++) {
if (!js_array->Get(i)->IsString())
js_array->Set(i, js_array->Get(i)->ToString(env()->isolate()));
auto value = js_array->Get(context, i).ToLocalChecked();

if (!value->IsString())
js_array->Set(context, i, value->ToString(env()->isolate())).FromJust();
}

// Index has a pointer to every string element, plus one more for a final
Expand All @@ -999,7 +1034,8 @@ int SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
// after every string. Align strings to cache lines.
data_size = 0;
for (uint32_t i = 0; i < length; i++) {
data_size += StringBytes::StorageSize(isolate, js_array->Get(i), UTF8) + 1;
auto value = js_array->Get(context, i).ToLocalChecked();
data_size += StringBytes::StorageSize(isolate, value, UTF8) + 1;
data_size = ROUND_UP(data_size, sizeof(void*));
}

Expand All @@ -1010,10 +1046,11 @@ int SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,

for (uint32_t i = 0; i < length; i++) {
list[i] = buffer + data_offset;
auto value = js_array->Get(context, i).ToLocalChecked();
data_offset += StringBytes::Write(isolate,
buffer + data_offset,
-1,
js_array->Get(i),
value,
UTF8);
buffer[data_offset++] = '\0';
data_offset = ROUND_UP(data_offset, sizeof(void*));
Expand Down