From ac7b5a2517a06cb8180b402e7c89971a42cd85cb Mon Sep 17 00:00:00 2001 From: Dmitrii Kuvaiskii Date: Mon, 29 Jun 2020 16:53:14 +0000 Subject: [PATCH] [Pal] Return POLLHUP on closed FDs in _DkStreamsWaitEvents() --- LibOS/shim/test/regression/.gitignore | 1 + LibOS/shim/test/regression/Makefile | 1 + LibOS/shim/test/regression/poll_closed_fd.c | 83 +++++++++++++++++++++ LibOS/shim/test/regression/test_libos.py | 6 ++ Pal/src/host/Linux-SGX/db_object.c | 19 ++++- Pal/src/host/Linux/db_object.c | 19 ++++- 6 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 LibOS/shim/test/regression/poll_closed_fd.c diff --git a/LibOS/shim/test/regression/.gitignore b/LibOS/shim/test/regression/.gitignore index 832d56c4..4fc14013 100644 --- a/LibOS/shim/test/regression/.gitignore +++ b/LibOS/shim/test/regression/.gitignore @@ -49,6 +49,7 @@ /openmp /pipe /poll +/poll_closed_fd /poll_many_types /ppoll /proc_common diff --git a/LibOS/shim/test/regression/Makefile b/LibOS/shim/test/regression/Makefile index 5ec8415a..7d081714 100644 --- a/LibOS/shim/test/regression/Makefile +++ b/LibOS/shim/test/regression/Makefile @@ -40,6 +40,7 @@ c_executables = \ openmp \ pipe \ poll \ + poll_closed_fd \ poll_many_types \ ppoll \ proc_common \ diff --git a/LibOS/shim/test/regression/poll_closed_fd.c b/LibOS/shim/test/regression/poll_closed_fd.c new file mode 100644 index 00000000..ea57c86c --- /dev/null +++ b/LibOS/shim/test/regression/poll_closed_fd.c @@ -0,0 +1,83 @@ +#include +#include +#include +#include +#include +#include + +int main(int argc, char** argv) { + int ret; + int pipefds[2]; + char buffer[1024]; + size_t bufsize = sizeof(buffer); + ssize_t bytes; + + if (pipe(pipefds) < 0) { + perror("pipe error\n"); + return 1; + } + + int pid = fork(); + + if (pid < 0) { + perror("fork error\n"); + return 1; + } else if (pid == 0) { + /* client */ + close(pipefds[0]); + + snprintf(buffer, bufsize, "Hello from write end of pipe!"); + if (write(pipefds[1], &buffer, strlen(buffer) + 1) < 0) { + perror("write error\n"); + close(pipefds[1]); + return 1; + } + close(pipefds[1]); + } else { + /* server */ + close(pipefds[1]); + + struct pollfd infds[] = { + {.fd = pipefds[0], .events = POLLIN}, + }; + /* parent (server) expects to receive one message from client (via POLLIN) and + * then get an error (via POLLHUP) because the client connection was closed */ + for (;;) { + ret = poll(infds, 1, -1); + if (ret <= 0) { + perror("poll with POLLIN failed\n"); + close(pipefds[0]); + return 1; + } + + if (infds[0].revents & POLLIN) { + bytes = read(pipefds[0], &buffer, bufsize - 1); + if (bytes < 0) { + perror("read error\n"); + close(pipefds[0]); + return 1; + } else if (bytes > 0) { + buffer[bytes] = '\0'; + printf("read on pipe: %s\n", buffer); + } + } + if (infds[0].revents & (POLLHUP | POLLERR | POLLNVAL)) { + printf("the peer closed its end of the pipe\n"); + break; + } + } + int wstatus; + if (wait(&wstatus) < 0) { + perror("wait error\n"); + close(pipefds[0]); + return 1; + } else if (!WIFEXITED(wstatus) || WEXITSTATUS(wstatus)){ + perror("child process didn't exit successfully\n"); + close(pipefds[0]); + return 1; + } + close(pipefds[0]); + } + + return 0; +} diff --git a/LibOS/shim/test/regression/test_libos.py b/LibOS/shim/test/regression/test_libos.py index 20fe4deb..f35a6847 100644 --- a/LibOS/shim/test/regression/test_libos.py +++ b/LibOS/shim/test/regression/test_libos.py @@ -532,6 +532,12 @@ class TC_80_Socket(RegressionTestCase): stdout, _ = self.run_binary(['poll_many_types']) self.assertIn('poll(POLLIN) returned 3 file descriptors', stdout) + def test_022_poll_closed_fd(self): + stdout, _ = self.run_binary(['poll_closed_fd'], timeout=60) + self.assertNotIn('poll with POLLIN failed', stdout) + self.assertIn('read on pipe: Hello from write end of pipe!', stdout) + self.assertIn('the peer closed its end of the pipe', stdout) + def test_030_ppoll(self): stdout, _ = self.run_binary(['ppoll']) self.assertIn('ppoll(POLLOUT) returned 1 file descriptors', stdout) diff --git a/Pal/src/host/Linux-SGX/db_object.c b/Pal/src/host/Linux-SGX/db_object.c index 8c4b69a3..cfcb8816 100644 --- a/Pal/src/host/Linux-SGX/db_object.c +++ b/Pal/src/host/Linux-SGX/db_object.c @@ -55,6 +55,7 @@ int _DkStreamsWaitEvents(size_t count, PAL_HANDLE* handle_array, PAL_FLG* events /* collect all FDs of all PAL handles that may report read/write events */ size_t nfds = 0; + size_t ret_events_updated = 0; for (size_t i = 0; i < count; i++) { ret_events[i] = 0; @@ -69,8 +70,15 @@ int _DkStreamsWaitEvents(size_t count, PAL_HANDLE* handle_array, PAL_FLG* events /* hdl might be a mutex/event/non-pollable object, simply ignore it */ if (hdl->generic.fds[j] == PAL_IDX_POISON) continue; - if (flags & ERROR(j)) + if (flags & ERROR(j)) { + /* PAL handle is requested for read/write but already marked with error: + * skip it but update its ret_events */ + if (events[i] & (PAL_WAIT_READ | PAL_WAIT_WRITE)) { + ret_events[i] |= PAL_WAIT_ERROR; + ret_events_updated++; + } continue; + } int fdevents = 0; fdevents |= ((flags & RFD(j)) && (events[i] & PAL_WAIT_READ)) ? POLLIN : 0; @@ -87,8 +95,13 @@ int _DkStreamsWaitEvents(size_t count, PAL_HANDLE* handle_array, PAL_FLG* events } if (!nfds) { - /* did not find any waitable FDs (LibOS supplied closed/errored FDs or empty events) */ - ret = -PAL_ERROR_TRYAGAIN; + if (ret_events_updated > 0) { + /* we skip actual ppoll, but there was at least one PAL handle with updated ret_events */ + ret = 0; + } else { + /* did not find any waitable FDs (LibOS supplied closed/errored FDs or empty events) */ + ret = -PAL_ERROR_TRYAGAIN; + } goto out; } diff --git a/Pal/src/host/Linux/db_object.c b/Pal/src/host/Linux/db_object.c index dbdef023..0c5a998d 100644 --- a/Pal/src/host/Linux/db_object.c +++ b/Pal/src/host/Linux/db_object.c @@ -55,6 +55,7 @@ int _DkStreamsWaitEvents(size_t count, PAL_HANDLE* handle_array, PAL_FLG* events /* collect all FDs of all PAL handles that may report read/write events */ size_t nfds = 0; + size_t ret_events_updated = 0; for (size_t i = 0; i < count; i++) { ret_events[i] = 0; @@ -69,8 +70,15 @@ int _DkStreamsWaitEvents(size_t count, PAL_HANDLE* handle_array, PAL_FLG* events /* hdl might be a mutex/event/non-pollable object, simply ignore it */ if (hdl->generic.fds[j] == PAL_IDX_POISON) continue; - if (flags & ERROR(j)) + if (flags & ERROR(j)) { + /* PAL handle is requested for read/write but already marked with error: + * skip it but update its ret_events */ + if (events[i] & (PAL_WAIT_READ | PAL_WAIT_WRITE)) { + ret_events[i] |= PAL_WAIT_ERROR; + ret_events_updated++; + } continue; + } int fdevents = 0; fdevents |= ((flags & RFD(j)) && (events[i] & PAL_WAIT_READ)) ? POLLIN : 0; @@ -87,8 +95,13 @@ int _DkStreamsWaitEvents(size_t count, PAL_HANDLE* handle_array, PAL_FLG* events } if (!nfds) { - /* did not find any waitable FDs (LibOS supplied closed/errored FDs or empty events) */ - ret = -PAL_ERROR_TRYAGAIN; + if (ret_events_updated > 0) { + /* we skip actual ppoll, but there was at least one PAL handle with updated ret_events */ + ret = 0; + } else { + /* did not find any waitable FDs (LibOS supplied closed/errored FDs or empty events) */ + ret = -PAL_ERROR_TRYAGAIN; + } goto out; }