[dart:io] [vm] Fix poll(2) loop while waiting for a process to exit.

The loop didn't correctly handle if the fd closed wasn't the last in the
list, instead it would skip an entry and would accidentally consider the
closed entry twice.

The error handling was also wrong, where a file descriptor could be
closed multiple times if an error occured after one of the file
descriptors had already been closed. Additionally the POLLERR and
POLLNVAL conditions were not handled.

Fixes https://github.com/dart-lang/sdk/issues/40441.

Change-Id: I4b4da067bbaf40af329e37ccf45d85f8bbf6c914
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/134723
Commit-Queue: Jonas Termansen <sortie@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This commit is contained in:
Jonas Termansen 2020-02-06 16:40:32 +00:00 committed by commit-bot@chromium.org
parent 4a76bafa5a
commit f688645909
3 changed files with 49 additions and 31 deletions

View file

@ -805,11 +805,11 @@ int Process::Start(Namespace* namespc,
return starter.Start();
}
static bool CloseProcessBuffers(struct pollfd fds[3]) {
static bool CloseProcessBuffers(struct pollfd* fds, int alive) {
int e = errno;
close(fds[0].fd);
close(fds[1].fd);
close(fds[2].fd);
for (int i = 0; i < alive; i++) {
close(fds[i].fd);
}
errno = e;
return false;
}
@ -846,28 +846,30 @@ bool Process::Wait(intptr_t pid,
while (alive > 0) {
// Blocking call waiting for events from the child process.
if (TEMP_FAILURE_RETRY(poll(fds, alive, -1)) <= 0) {
return CloseProcessBuffers(fds);
return CloseProcessBuffers(fds, alive);
}
// Process incoming data.
int current_alive = alive;
for (int i = 0; i < current_alive; i++) {
for (int i = 0; i < alive; i++) {
if ((fds[i].revents & (POLLNVAL | POLLERR)) != 0) {
return CloseProcessBuffers(fds, alive);
}
if ((fds[i].revents & POLLIN) != 0) {
intptr_t avail = FDUtils::AvailableBytes(fds[i].fd);
if (fds[i].fd == out) {
if (!out_data.Read(out, avail)) {
return CloseProcessBuffers(fds);
return CloseProcessBuffers(fds, alive);
}
} else if (fds[i].fd == err) {
if (!err_data.Read(err, avail)) {
return CloseProcessBuffers(fds);
return CloseProcessBuffers(fds, alive);
}
} else if (fds[i].fd == exit_event) {
if (avail == 8) {
intptr_t b =
TEMP_FAILURE_RETRY(read(exit_event, exit_code_data.bytes, 8));
if (b != 8) {
return CloseProcessBuffers(fds);
return CloseProcessBuffers(fds, alive);
}
}
} else {
@ -875,11 +877,15 @@ bool Process::Wait(intptr_t pid,
}
}
if ((fds[i].revents & POLLHUP) != 0) {
// Remove the pollfd from the list of pollfds.
close(fds[i].fd);
alive--;
if (i < alive) {
fds[i] = fds[alive];
}
// Process the same index again.
i--;
continue;
}
}
}

View file

@ -804,11 +804,11 @@ int Process::Start(Namespace* namespc,
return starter.Start();
}
static bool CloseProcessBuffers(struct pollfd fds[3]) {
static bool CloseProcessBuffers(struct pollfd* fds, int alive) {
int e = errno;
close(fds[0].fd);
close(fds[1].fd);
close(fds[2].fd);
for (int i = 0; i < alive; i++) {
close(fds[i].fd);
}
errno = e;
return false;
}
@ -845,28 +845,30 @@ bool Process::Wait(intptr_t pid,
while (alive > 0) {
// Blocking call waiting for events from the child process.
if (TEMP_FAILURE_RETRY(poll(fds, alive, -1)) <= 0) {
return CloseProcessBuffers(fds);
return CloseProcessBuffers(fds, alive);
}
// Process incoming data.
int current_alive = alive;
for (int i = 0; i < current_alive; i++) {
for (int i = 0; i < alive; i++) {
if ((fds[i].revents & (POLLNVAL | POLLERR)) != 0) {
return CloseProcessBuffers(fds, alive);
}
if ((fds[i].revents & POLLIN) != 0) {
intptr_t avail = FDUtils::AvailableBytes(fds[i].fd);
if (fds[i].fd == out) {
if (!out_data.Read(out, avail)) {
return CloseProcessBuffers(fds);
return CloseProcessBuffers(fds, alive);
}
} else if (fds[i].fd == err) {
if (!err_data.Read(err, avail)) {
return CloseProcessBuffers(fds);
return CloseProcessBuffers(fds, alive);
}
} else if (fds[i].fd == exit_event) {
if (avail == 8) {
intptr_t b =
TEMP_FAILURE_RETRY(read(exit_event, exit_code_data.bytes, 8));
if (b != 8) {
return CloseProcessBuffers(fds);
return CloseProcessBuffers(fds, alive);
}
}
} else {
@ -874,11 +876,15 @@ bool Process::Wait(intptr_t pid,
}
}
if ((fds[i].revents & POLLHUP) != 0) {
// Remove the pollfd from the list of pollfds.
close(fds[i].fd);
alive--;
if (i < alive) {
fds[i] = fds[alive];
}
// Process the same index again.
i--;
continue;
}
}
}

View file

@ -770,11 +770,11 @@ int Process::Start(Namespace* namespc,
return starter.Start();
}
static bool CloseProcessBuffers(struct pollfd fds[3]) {
static bool CloseProcessBuffers(struct pollfd* fds, int alive) {
int e = errno;
close(fds[0].fd);
close(fds[1].fd);
close(fds[2].fd);
for (int i = 0; i < alive; i++) {
close(fds[i].fd);
}
errno = e;
return false;
}
@ -811,13 +811,15 @@ bool Process::Wait(intptr_t pid,
while (alive > 0) {
// Blocking call waiting for events from the child process.
if (TEMP_FAILURE_RETRY(poll(fds, alive, -1)) <= 0) {
return CloseProcessBuffers(fds);
return CloseProcessBuffers(fds, alive);
}
// Process incoming data.
int current_alive = alive;
for (int i = 0; i < current_alive; i++) {
for (int i = 0; i < alive; i++) {
intptr_t avail;
if ((fds[i].revents & (POLLNVAL | POLLERR)) != 0) {
return CloseProcessBuffers(fds, alive);
}
if ((fds[i].revents & POLLIN) != 0) {
avail = FDUtils::AvailableBytes(fds[i].fd);
// On Mac OS POLLIN can be set with zero available
@ -825,18 +827,18 @@ bool Process::Wait(intptr_t pid,
if (avail > 0) {
if (fds[i].fd == out) {
if (!out_data.Read(out, avail)) {
return CloseProcessBuffers(fds);
return CloseProcessBuffers(fds, alive);
}
} else if (fds[i].fd == err) {
if (!err_data.Read(err, avail)) {
return CloseProcessBuffers(fds);
return CloseProcessBuffers(fds, alive);
}
} else if (fds[i].fd == exit_event) {
if (avail == 8) {
intptr_t b =
TEMP_FAILURE_RETRY(read(fds[i].fd, exit_code_data.bytes, 8));
TEMP_FAILURE_RETRY(read(exit_event, exit_code_data.bytes, 8));
if (b != 8) {
return CloseProcessBuffers(fds);
return CloseProcessBuffers(fds, alive);
}
}
} else {
@ -846,11 +848,15 @@ bool Process::Wait(intptr_t pid,
}
if (((fds[i].revents & POLLHUP) != 0) ||
(((fds[i].revents & POLLIN) != 0) && (avail == 0))) {
// Remove the pollfd from the list of pollfds.
close(fds[i].fd);
alive--;
if (i < alive) {
fds[i] = fds[alive];
}
// Process the same index again.
i--;
continue;
}
}
}