From 2daa88cfb464c08505d8f3579191b134f3417deb Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 1 Jul 2015 14:46:42 -0700 Subject: [PATCH] Fix a TSAN reported race close() could race with epoll_ctl(); pretend to be polling while adding to the epoll set to prevent this --- src/core/iomgr/fd_posix.c | 4 +-- .../iomgr/pollset_multipoller_with_epoll.c | 26 ++++++++++++------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/core/iomgr/fd_posix.c b/src/core/iomgr/fd_posix.c index bcc11f91b70..1297145d1a7 100644 --- a/src/core/iomgr/fd_posix.c +++ b/src/core/iomgr/fd_posix.c @@ -371,13 +371,13 @@ gpr_uint32 grpc_fd_begin_poll(grpc_fd *fd, grpc_pollset *pollset, return 0; } /* if there is nobody polling for read, but we need to, then start doing so */ - if (!fd->read_watcher && gpr_atm_acq_load(&fd->readst) > READY) { + if (read_mask && !fd->read_watcher && gpr_atm_acq_load(&fd->readst) > READY) { fd->read_watcher = watcher; mask |= read_mask; } /* if there is nobody polling for write, but we need to, then start doing so */ - if (!fd->write_watcher && gpr_atm_acq_load(&fd->writest) > READY) { + if (write_mask && !fd->write_watcher && gpr_atm_acq_load(&fd->writest) > READY) { fd->write_watcher = watcher; mask |= write_mask; } diff --git a/src/core/iomgr/pollset_multipoller_with_epoll.c b/src/core/iomgr/pollset_multipoller_with_epoll.c index dcf08d379cb..1900bbf9e14 100644 --- a/src/core/iomgr/pollset_multipoller_with_epoll.c +++ b/src/core/iomgr/pollset_multipoller_with_epoll.c @@ -54,17 +54,25 @@ static void multipoll_with_epoll_pollset_add_fd(grpc_pollset *pollset, pollset_hdr *h = pollset->data.ptr; struct epoll_event ev; int err; - - ev.events = EPOLLIN | EPOLLOUT | EPOLLET; - ev.data.ptr = fd; - err = epoll_ctl(h->epoll_fd, EPOLL_CTL_ADD, fd->fd, &ev); - if (err < 0) { - /* FDs may be added to a pollset multiple times, so EEXIST is normal. */ - if (errno != EEXIST) { - gpr_log(GPR_ERROR, "epoll_ctl add for %d failed: %s", fd->fd, - strerror(errno)); + grpc_fd_watcher watcher; + + /* We pretend to be polling whilst adding an fd to keep the fd from being + closed during the add. This may result in a spurious wakeup being assigned + to this pollset whilst adding, but that should be benign. */ + GPR_ASSERT(grpc_fd_begin_poll(fd, pollset, 0, 0, &watcher) == 0); + if (watcher.fd != NULL) { + ev.events = EPOLLIN | EPOLLOUT | EPOLLET; + ev.data.ptr = fd; + err = epoll_ctl(h->epoll_fd, EPOLL_CTL_ADD, fd->fd, &ev); + if (err < 0) { + /* FDs may be added to a pollset multiple times, so EEXIST is normal. */ + if (errno != EEXIST) { + gpr_log(GPR_ERROR, "epoll_ctl add for %d failed: %s", fd->fd, + strerror(errno)); + } } } + grpc_fd_end_poll(&watcher, 0, 0); } static void multipoll_with_epoll_pollset_del_fd(grpc_pollset *pollset,