Skip to content

Commit a1a9957

Browse files
fhinkelMylesBorins
authored andcommitted
src: make ownership of stdio_pipes explicit
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 <[email protected]>
1 parent 7329537 commit a1a9957

File tree

2 files changed

+17
-25
lines changed

2 files changed

+17
-25
lines changed

src/spawn_sync.cc

+16-24
Original file line numberDiff line numberDiff line change
@@ -417,14 +417,7 @@ SyncProcessRunner::SyncProcessRunner(Environment* env)
417417
SyncProcessRunner::~SyncProcessRunner() {
418418
CHECK_EQ(lifecycle_, kHandlesClosed);
419419

420-
if (stdio_pipes_ != nullptr) {
421-
for (size_t i = 0; i < stdio_count_; i++) {
422-
if (stdio_pipes_[i] != nullptr)
423-
delete stdio_pipes_[i];
424-
}
425-
}
426-
427-
delete[] stdio_pipes_;
420+
stdio_pipes_.reset();
428421
delete[] file_buffer_;
429422
delete[] args_buffer_;
430423
delete[] cwd_buffer_;
@@ -495,7 +488,7 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
495488
uv_process_.data = this;
496489

497490
for (uint32_t i = 0; i < stdio_count_; i++) {
498-
SyncProcessStdioPipe* h = stdio_pipes_[i];
491+
SyncProcessStdioPipe* h = stdio_pipes_[i].get();
499492
if (h != nullptr) {
500493
r = h->Start();
501494
if (r < 0)
@@ -554,11 +547,11 @@ void SyncProcessRunner::CloseStdioPipes() {
554547
CHECK_LT(lifecycle_, kHandlesClosed);
555548

556549
if (stdio_pipes_initialized_) {
557-
CHECK_NE(stdio_pipes_, nullptr);
550+
CHECK(stdio_pipes_);
558551
CHECK_NE(uv_loop_, nullptr);
559552

560553
for (uint32_t i = 0; i < stdio_count_; i++) {
561-
if (stdio_pipes_[i] != nullptr)
554+
if (stdio_pipes_[i])
562555
stdio_pipes_[i]->Close();
563556
}
564557

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

712705
Local<Array> SyncProcessRunner::BuildOutputArray() {
713706
CHECK_GE(lifecycle_, kInitialized);
714-
CHECK_NE(stdio_pipes_, nullptr);
707+
CHECK(stdio_pipes_);
715708

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

720713
for (uint32_t i = 0; i < stdio_count_; i++) {
721-
SyncProcessStdioPipe* h = stdio_pipes_[i];
714+
SyncProcessStdioPipe* h = stdio_pipes_[i].get();
722715
if (h != nullptr && h->writable())
723716
js_output->Set(context, i, h->GetOutputAsBuffer(env())).FromJust();
724717
else
@@ -851,7 +844,8 @@ int SyncProcessRunner::ParseStdioOptions(Local<Value> js_value) {
851844
stdio_count_ = js_stdio_options->Length();
852845
uv_stdio_containers_ = new uv_stdio_container_t[stdio_count_];
853846

854-
stdio_pipes_ = new SyncProcessStdioPipe*[stdio_count_]();
847+
stdio_pipes_.reset(
848+
new std::unique_ptr<SyncProcessStdioPipe>[stdio_count_]());
855849
stdio_pipes_initialized_ = true;
856850

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

926920
int SyncProcessRunner::AddStdioIgnore(uint32_t child_fd) {
927921
CHECK_LT(child_fd, stdio_count_);
928-
CHECK_EQ(stdio_pipes_[child_fd], nullptr);
922+
CHECK(!stdio_pipes_[child_fd]);
929923

930924
uv_stdio_containers_[child_fd].flags = UV_IGNORE;
931925

@@ -938,31 +932,29 @@ int SyncProcessRunner::AddStdioPipe(uint32_t child_fd,
938932
bool writable,
939933
uv_buf_t input_buffer) {
940934
CHECK_LT(child_fd, stdio_count_);
941-
CHECK_EQ(stdio_pipes_[child_fd], nullptr);
935+
CHECK(!stdio_pipes_[child_fd]);
942936

943-
SyncProcessStdioPipe* h = new SyncProcessStdioPipe(this,
944-
readable,
945-
writable,
946-
input_buffer);
937+
std::unique_ptr<SyncProcessStdioPipe> h(
938+
new SyncProcessStdioPipe(this, readable, writable, input_buffer));
947939

948940
int r = h->Initialize(uv_loop_);
949941
if (r < 0) {
950-
delete h;
942+
h.reset();
951943
return r;
952944
}
953945

954-
stdio_pipes_[child_fd] = h;
955-
956946
uv_stdio_containers_[child_fd].flags = h->uv_flags();
957947
uv_stdio_containers_[child_fd].data.stream = h->uv_stream();
958948

949+
stdio_pipes_[child_fd] = std::move(h);
950+
959951
return 0;
960952
}
961953

962954

963955
int SyncProcessRunner::AddStdioInheritFD(uint32_t child_fd, int inherit_fd) {
964956
CHECK_LT(child_fd, stdio_count_);
965-
CHECK_EQ(stdio_pipes_[child_fd], nullptr);
957+
CHECK(!stdio_pipes_[child_fd]);
966958

967959
uv_stdio_containers_[child_fd].flags = UV_INHERIT_FD;
968960
uv_stdio_containers_[child_fd].data.fd = inherit_fd;

src/spawn_sync.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ class SyncProcessRunner {
213213

214214
uint32_t stdio_count_;
215215
uv_stdio_container_t* uv_stdio_containers_;
216-
SyncProcessStdioPipe** stdio_pipes_;
216+
std::unique_ptr<std::unique_ptr<SyncProcessStdioPipe>[]> stdio_pipes_;
217217
bool stdio_pipes_initialized_;
218218

219219
uv_process_options_t uv_process_options_;

0 commit comments

Comments
 (0)