Skip to content

src: use RAII for uv_process_options_t #59945

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

Closed
wants to merge 2 commits into from
Closed

src: use RAII for uv_process_options_t #59945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0572379
src: use RAII for uv_process_options_t
iknoom Sep 20, 2025
985fed4
src: fill `options_args`, `options_env` after vectors are finalized
iknoom Sep 22, 2025
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
87 changes: 44 additions & 43 deletions src/process_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,10 @@ class ProcessWrap : public HandleWrap {
return Just(stream);
}

static Maybe<void> ParseStdioOptions(Environment* env,
Local<Object> js_options,
uv_process_options_t* options) {
static Maybe<void> ParseStdioOptions(
Environment* env,
Local<Object> js_options,
std::vector<uv_stdio_container_t>* options_stdio) {
Local<Context> context = env->context();
Local<String> stdio_key = env->stdio_string();
Local<Value> stdios_val;
Expand All @@ -132,8 +133,7 @@ class ProcessWrap : public HandleWrap {
Local<Array> stdios = stdios_val.As<Array>();

uint32_t len = stdios->Length();
options->stdio = new uv_stdio_container_t[len];
options->stdio_count = len;
options_stdio->resize(len);

for (uint32_t i = 0; i < len; i++) {
Local<Value> val;
Expand All @@ -147,23 +147,23 @@ class ProcessWrap : public HandleWrap {
}

if (type->StrictEquals(env->ignore_string())) {
options->stdio[i].flags = UV_IGNORE;
(*options_stdio)[i].flags = UV_IGNORE;
} else if (type->StrictEquals(env->pipe_string())) {
options->stdio[i].flags = static_cast<uv_stdio_flags>(
(*options_stdio)[i].flags = static_cast<uv_stdio_flags>(
UV_CREATE_PIPE | UV_READABLE_PIPE | UV_WRITABLE_PIPE);
if (!StreamForWrap(env, stdio).To(&options->stdio[i].data.stream)) {
if (!StreamForWrap(env, stdio).To(&(*options_stdio)[i].data.stream)) {
return Nothing<void>();
}
} else if (type->StrictEquals(env->overlapped_string())) {
options->stdio[i].flags = static_cast<uv_stdio_flags>(
UV_CREATE_PIPE | UV_READABLE_PIPE | UV_WRITABLE_PIPE |
UV_OVERLAPPED_PIPE);
if (!StreamForWrap(env, stdio).To(&options->stdio[i].data.stream)) {
(*options_stdio)[i].flags =
static_cast<uv_stdio_flags>(UV_CREATE_PIPE | UV_READABLE_PIPE |
UV_WRITABLE_PIPE | UV_OVERLAPPED_PIPE);
if (!StreamForWrap(env, stdio).To(&(*options_stdio)[i].data.stream)) {
return Nothing<void>();
}
} else if (type->StrictEquals(env->wrap_string())) {
options->stdio[i].flags = UV_INHERIT_STREAM;
if (!StreamForWrap(env, stdio).To(&options->stdio[i].data.stream)) {
(*options_stdio)[i].flags = UV_INHERIT_STREAM;
if (!StreamForWrap(env, stdio).To(&(*options_stdio)[i].data.stream)) {
return Nothing<void>();
}
} else {
Expand All @@ -174,8 +174,8 @@ class ProcessWrap : public HandleWrap {
}
CHECK(fd_value->IsNumber());
int fd = FromV8Value<int>(fd_value);
options->stdio[i].flags = UV_INHERIT_FD;
options->stdio[i].data.fd = fd;
(*options_stdio)[i].flags = UV_INHERIT_FD;
(*options_stdio)[i].data.fd = fd;
}
}
return JustVoid();
Expand All @@ -199,8 +199,6 @@ class ProcessWrap : public HandleWrap {

options.exit_cb = OnExit;

// TODO(bnoordhuis) is this possible to do without mallocing ?

// options.file
Local<Value> file_v;
if (!js_options->Get(context, env->file_string()).ToLocal(&file_v)) {
Expand Down Expand Up @@ -251,23 +249,28 @@ class ProcessWrap : public HandleWrap {
if (!js_options->Get(context, env->args_string()).ToLocal(&argv_v)) {
return;
}
if (!argv_v.IsEmpty() && argv_v->IsArray()) {
std::vector<char*> options_args;
std::vector<std::string> args_vals;
if (argv_v->IsArray()) {
Local<Array> js_argv = argv_v.As<Array>();
int argc = js_argv->Length();
CHECK_LT(argc, INT_MAX); // Check for overflow.

// Heap allocate to detect errors. +1 is for nullptr.
options.args = new char*[argc + 1];
args_vals.reserve(argc);
for (int i = 0; i < argc; i++) {
Local<Value> val;
if (!js_argv->Get(context, i).ToLocal(&val)) {
return;
}
node::Utf8Value arg(env->isolate(), val);
options.args[i] = strdup(*arg);
CHECK_NOT_NULL(options.args[i]);
args_vals.emplace_back(arg.ToString());
}
options_args.resize(args_vals.size() + 1);
for (size_t i = 0; i < args_vals.size(); i++) {
options_args[i] = const_cast<char*>(args_vals[i].c_str());
CHECK_NOT_NULL(options_args[i]);
}
options.args[argc] = nullptr;
options_args.back() = nullptr;
options.args = options_args.data();
}

// options.cwd
Expand All @@ -286,27 +289,37 @@ class ProcessWrap : public HandleWrap {
if (!js_options->Get(context, env->env_pairs_string()).ToLocal(&env_v)) {
return;
}
if (!env_v.IsEmpty() && env_v->IsArray()) {
std::vector<char*> options_env;
std::vector<std::string> env_vals;
if (env_v->IsArray()) {
Local<Array> env_opt = env_v.As<Array>();
int envc = env_opt->Length();
CHECK_LT(envc, INT_MAX); // Check for overflow.
options.env = new char*[envc + 1]; // Heap allocated to detect errors.
env_vals.reserve(envc);
for (int i = 0; i < envc; i++) {
Local<Value> val;
if (!env_opt->Get(context, i).ToLocal(&val)) {
return;
}
node::Utf8Value pair(env->isolate(), val);
options.env[i] = strdup(*pair);
CHECK_NOT_NULL(options.env[i]);
env_vals.emplace_back(pair.ToString());
}
options.env[envc] = nullptr;
options_env.resize(env_vals.size() + 1);
for (size_t i = 0; i < env_vals.size(); i++) {
options_env[i] = const_cast<char*>(env_vals[i].c_str());
CHECK_NOT_NULL(options_env[i]);
}
options_env.back() = nullptr;
options.env = options_env.data();
}

// options.stdio
if (ParseStdioOptions(env, js_options, &options).IsNothing()) {
std::vector<uv_stdio_container_t> options_stdio;
if (ParseStdioOptions(env, js_options, &options_stdio).IsNothing()) {
return;
}
options.stdio = options_stdio.data();
options.stdio_count = options_stdio.size();

// options.windowsHide
Local<Value> hide_v;
Expand Down Expand Up @@ -361,18 +374,6 @@ class ProcessWrap : public HandleWrap {
}
}

if (options.args) {
for (int i = 0; options.args[i]; i++) free(options.args[i]);
delete [] options.args;
}

if (options.env) {
for (int i = 0; options.env[i]; i++) free(options.env[i]);
delete [] options.env;
}

delete[] options.stdio;

args.GetReturnValue().Set(err);
}

Expand Down
44 changes: 19 additions & 25 deletions src/spawn_sync.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,6 @@ void SyncProcessRunner::Spawn(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(result);
}


SyncProcessRunner::SyncProcessRunner(Environment* env)
: max_buffer_(0),
timeout_(0),
Expand All @@ -412,14 +411,9 @@ SyncProcessRunner::SyncProcessRunner(Environment* env)
uv_loop_(nullptr),

stdio_count_(0),
uv_stdio_containers_(nullptr),
stdio_pipes_initialized_(false),

uv_process_options_(),
file_buffer_(nullptr),
args_buffer_(nullptr),
env_buffer_(nullptr),
cwd_buffer_(nullptr),

uv_process_(),
killed_(false),
Expand All @@ -436,19 +430,12 @@ SyncProcessRunner::SyncProcessRunner(Environment* env)

lifecycle_(kUninitialized),

env_(env) {
}

env_(env) {}

SyncProcessRunner::~SyncProcessRunner() {
CHECK_EQ(lifecycle_, kHandlesClosed);

stdio_pipes_.clear();
delete[] file_buffer_;
delete[] args_buffer_;
delete[] cwd_buffer_;
delete[] env_buffer_;
delete[] uv_stdio_containers_;
}


Expand Down Expand Up @@ -808,12 +795,14 @@ Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {
Local<Object> js_options = js_value.As<Object>();

Local<Value> js_file;
const char* file_buffer;
if (!js_options->Get(context, env()->file_string()).ToLocal(&js_file) ||
!CopyJsString(js_file, &file_buffer_).To(&r)) {
!CopyJsString(js_file, &file_buffer).To(&r)) {
return Nothing<int>();
}
if (r < 0) return Just(r);
uv_process_options_.file = file_buffer_;
file_buffer_.reset(file_buffer);
uv_process_options_.file = file_buffer_.get();

// Undocumented feature of Win32 CreateProcess API allows spawning
// batch files directly but is potentially insecure because arguments
Expand All @@ -825,23 +814,27 @@ Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {
#endif

Local<Value> js_args;
char* args_buffer;
if (!js_options->Get(context, env()->args_string()).ToLocal(&js_args) ||
!CopyJsStringArray(js_args, &args_buffer_).To(&r)) {
!CopyJsStringArray(js_args, &args_buffer).To(&r)) {
return Nothing<int>();
}
if (r < 0) return Just(r);
uv_process_options_.args = reinterpret_cast<char**>(args_buffer_);
args_buffer_.reset(args_buffer);
uv_process_options_.args = reinterpret_cast<char**>(args_buffer_.get());

Local<Value> js_cwd;
if (!js_options->Get(context, env()->cwd_string()).ToLocal(&js_cwd)) {
return Nothing<int>();
}
if (!js_cwd->IsNullOrUndefined()) {
if (!CopyJsString(js_cwd, &cwd_buffer_).To(&r)) {
const char* cwd_buffer;
if (!CopyJsString(js_cwd, &cwd_buffer).To(&r)) {
return Nothing<int>();
}
if (r < 0) return Just(r);
uv_process_options_.cwd = cwd_buffer_;
cwd_buffer_.reset(cwd_buffer);
uv_process_options_.cwd = cwd_buffer_.get();
}

Local<Value> js_env_pairs;
Expand All @@ -850,12 +843,13 @@ Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {
return Nothing<int>();
}
if (!js_env_pairs->IsNullOrUndefined()) {
if (!CopyJsStringArray(js_env_pairs, &env_buffer_).To(&r)) {
char* env_buffer;
if (!CopyJsStringArray(js_env_pairs, &env_buffer).To(&r)) {
return Nothing<int>();
}
if (r < 0) return Just(r);

uv_process_options_.env = reinterpret_cast<char**>(env_buffer_);
env_buffer_.reset(env_buffer);
uv_process_options_.env = reinterpret_cast<char**>(env_buffer_.get());
}
Local<Value> js_uid;
if (!js_options->Get(context, env()->uid_string()).ToLocal(&js_uid)) {
Expand Down Expand Up @@ -982,7 +976,7 @@ Maybe<int> SyncProcessRunner::ParseStdioOptions(Local<Value> js_value) {
js_stdio_options = js_value.As<Array>();

stdio_count_ = js_stdio_options->Length();
uv_stdio_containers_ = new uv_stdio_container_t[stdio_count_];
uv_stdio_containers_.resize(stdio_count_);

stdio_pipes_.clear();
stdio_pipes_.resize(stdio_count_);
Expand All @@ -1007,7 +1001,7 @@ Maybe<int> SyncProcessRunner::ParseStdioOptions(Local<Value> js_value) {
}
}

uv_process_options_.stdio = uv_stdio_containers_;
uv_process_options_.stdio = uv_stdio_containers_.data();
uv_process_options_.stdio_count = stdio_count_;

return Just<int>(0);
Expand Down
10 changes: 5 additions & 5 deletions src/spawn_sync.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,15 @@ class SyncProcessRunner {
uv_loop_t* uv_loop_;

uint32_t stdio_count_;
uv_stdio_container_t* uv_stdio_containers_;
std::vector<uv_stdio_container_t> uv_stdio_containers_;
std::vector<std::unique_ptr<SyncProcessStdioPipe>> stdio_pipes_;
bool stdio_pipes_initialized_;

uv_process_options_t uv_process_options_;
const char* file_buffer_;
char* args_buffer_;
char* env_buffer_;
const char* cwd_buffer_;
std::unique_ptr<const char[]> file_buffer_;
std::unique_ptr<char[]> args_buffer_;
std::unique_ptr<char[]> env_buffer_;
std::unique_ptr<const char[]> cwd_buffer_;

uv_process_t uv_process_;
bool killed_;
Expand Down
Loading