Skip to content

Commit

Permalink
src: make ownership of stdio_pipes explicit
Browse files Browse the repository at this point in the history
Use smart pointers instead of raw pointers for StdioPipes.
Also, use a smart pointer for the array holding them.

PR-URL: #17030
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
fhinkel authored and MylesBorins committed Dec 11, 2017
1 parent 7329537 commit a1a9957
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 25 deletions.
40 changes: 16 additions & 24 deletions src/spawn_sync.cc
Original file line number Diff line number Diff line change
Expand Up @@ -417,14 +417,7 @@ SyncProcessRunner::SyncProcessRunner(Environment* env)
SyncProcessRunner::~SyncProcessRunner() {
CHECK_EQ(lifecycle_, kHandlesClosed);

if (stdio_pipes_ != nullptr) {
for (size_t i = 0; i < stdio_count_; i++) {
if (stdio_pipes_[i] != nullptr)
delete stdio_pipes_[i];
}
}

delete[] stdio_pipes_;
stdio_pipes_.reset();
delete[] file_buffer_;
delete[] args_buffer_;
delete[] cwd_buffer_;
Expand Down Expand Up @@ -495,7 +488,7 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
uv_process_.data = this;

for (uint32_t i = 0; i < stdio_count_; i++) {
SyncProcessStdioPipe* h = stdio_pipes_[i];
SyncProcessStdioPipe* h = stdio_pipes_[i].get();
if (h != nullptr) {
r = h->Start();
if (r < 0)
Expand Down Expand Up @@ -554,11 +547,11 @@ void SyncProcessRunner::CloseStdioPipes() {
CHECK_LT(lifecycle_, kHandlesClosed);

if (stdio_pipes_initialized_) {
CHECK_NE(stdio_pipes_, nullptr);
CHECK(stdio_pipes_);
CHECK_NE(uv_loop_, nullptr);

for (uint32_t i = 0; i < stdio_count_; i++) {
if (stdio_pipes_[i] != nullptr)
if (stdio_pipes_[i])
stdio_pipes_[i]->Close();
}

Expand Down Expand Up @@ -711,14 +704,14 @@ Local<Object> SyncProcessRunner::BuildResultObject() {

Local<Array> SyncProcessRunner::BuildOutputArray() {
CHECK_GE(lifecycle_, kInitialized);
CHECK_NE(stdio_pipes_, nullptr);
CHECK(stdio_pipes_);

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];
SyncProcessStdioPipe* h = stdio_pipes_[i].get();
if (h != nullptr && h->writable())
js_output->Set(context, i, h->GetOutputAsBuffer(env())).FromJust();
else
Expand Down Expand Up @@ -851,7 +844,8 @@ int SyncProcessRunner::ParseStdioOptions(Local<Value> js_value) {
stdio_count_ = js_stdio_options->Length();
uv_stdio_containers_ = new uv_stdio_container_t[stdio_count_];

stdio_pipes_ = new SyncProcessStdioPipe*[stdio_count_]();
stdio_pipes_.reset(
new std::unique_ptr<SyncProcessStdioPipe>[stdio_count_]());
stdio_pipes_initialized_ = true;

for (uint32_t i = 0; i < stdio_count_; i++) {
Expand Down Expand Up @@ -925,7 +919,7 @@ int SyncProcessRunner::ParseStdioOption(int child_fd,

int SyncProcessRunner::AddStdioIgnore(uint32_t child_fd) {
CHECK_LT(child_fd, stdio_count_);
CHECK_EQ(stdio_pipes_[child_fd], nullptr);
CHECK(!stdio_pipes_[child_fd]);

uv_stdio_containers_[child_fd].flags = UV_IGNORE;

Expand All @@ -938,31 +932,29 @@ int SyncProcessRunner::AddStdioPipe(uint32_t child_fd,
bool writable,
uv_buf_t input_buffer) {
CHECK_LT(child_fd, stdio_count_);
CHECK_EQ(stdio_pipes_[child_fd], nullptr);
CHECK(!stdio_pipes_[child_fd]);

SyncProcessStdioPipe* h = new SyncProcessStdioPipe(this,
readable,
writable,
input_buffer);
std::unique_ptr<SyncProcessStdioPipe> h(
new SyncProcessStdioPipe(this, readable, writable, input_buffer));

int r = h->Initialize(uv_loop_);
if (r < 0) {
delete h;
h.reset();
return r;
}

stdio_pipes_[child_fd] = h;

uv_stdio_containers_[child_fd].flags = h->uv_flags();
uv_stdio_containers_[child_fd].data.stream = h->uv_stream();

stdio_pipes_[child_fd] = std::move(h);

return 0;
}


int SyncProcessRunner::AddStdioInheritFD(uint32_t child_fd, int inherit_fd) {
CHECK_LT(child_fd, stdio_count_);
CHECK_EQ(stdio_pipes_[child_fd], nullptr);
CHECK(!stdio_pipes_[child_fd]);

uv_stdio_containers_[child_fd].flags = UV_INHERIT_FD;
uv_stdio_containers_[child_fd].data.fd = inherit_fd;
Expand Down
2 changes: 1 addition & 1 deletion src/spawn_sync.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ class SyncProcessRunner {

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

uv_process_options_t uv_process_options_;
Expand Down

0 comments on commit a1a9957

Please sign in to comment.