Ensure no epoll events are processed for closed peer files (#788)

An error existed in transport.cc where it incorrectly assumed that
it would not receive epoll events relating to a closed peer
connection. This is not the case because file descriptors can be
shared between processes e.g. on exec. When they are, events could
still be generated. The solution is to explicitly de-register our
interest in the file descriptor before we close it using
epoll_ctl(EPOLL_CTL_DEL). In addition the call to accept in
listener.cc has been updated to set CLOEXEC on the peer file
descriptors as these should not be shared with other processes
anyway.
Co-authored-by: Roberts, Chris (UK Gloucester) <chris.v.roberts@baesystems.com>

include/pistache/reactor.h

@@ -120,6 +120,8 @@ public:
   void modifyFd(const Key &key, Fd fd, Polling::NotifyOn interest,
                 Polling::Tag tag, Polling::Mode mode = Polling::Mode::Level);
 
+  void removeFd(const Key& key, Fd fd);
+
   void runOnce();
   void run();
 

src/common/reactor.cc

@@ -43,6 +43,8 @@ public:
                         Polling::NotifyOn interest, Polling::Tag tag,
                         Polling::Mode mode = Polling::Mode::Level) = 0;
 
+  virtual void removeFd(const Reactor::Key& key, Fd fd) = 0;
+
   virtual void runOnce() = 0;
   virtual void run() = 0;
 
@@ -112,6 +114,10 @@ public:
     poller.rearmFd(fd, Flags<Polling::NotifyOn>(interest), pollTag, mode);
   }
 
+  void removeFd(const Reactor::Key& key, Fd fd) override {
+    poller.removeFd(fd);
+  }
+
   void runOnce() override {
     if (handlers_.empty())
       throw std::runtime_error("You need to set at least one handler");
@@ -375,6 +381,10 @@ public:
     dispatchCall(key, &SyncImpl::modifyFd, fd, interest, tag, mode);
   }
 
+  void removeFd(const Reactor::Key& key, Fd fd) override {
+    dispatchCall(key, &SyncImpl::removeFd, fd);
+  }
+
   void runOnce() override {}
 
   void run() override {
@@ -511,6 +521,10 @@ void Reactor::modifyFd(const Reactor::Key &key, Fd fd,
   impl()->modifyFd(key, fd, interest, Polling::Tag(fd), mode);
 }
 
+void Reactor::removeFd(const Reactor::Key& key, Fd fd) {
+  impl()->removeFd(key, fd);
+}
+
 void Reactor::run() { impl()->run(); }
 
 void Reactor::shutdown() {

src/common/transport.cc

@@ -173,6 +173,14 @@ void Transport::handlePeerDisconnection(const std::shared_ptr<Peer> &peer) {
     toWrite.erase(fd);
   }
 
+  // Don't rely on close deleting this FD from the epoll "interest" list.
+  // This is needed in case the FD has been shared with another process.
+  // Sharing should no longer happen by accident as SOCK_CLOEXEC is now set on
+  // listener accept. This should then guarantee that the next call to
+  // epoll_wait will not give us any events relating to this FD even if they
+  // have been queued in the kernel since the last call to epoll_wait.
+  reactor()->removeFd(key(), fd);
+
   close(fd);
 }
 

src/server/listener.cc

@@ -427,8 +427,9 @@ void Listener::handleNewConnection() {
 
 int Listener::acceptConnection(struct sockaddr_in &peer_addr) const {
   socklen_t peer_addr_len = sizeof(peer_addr);
-  int client_fd =
-      ::accept(listen_fd, (struct sockaddr *)&peer_addr, &peer_addr_len);
+  // Do not share open FD with forked processes
+  int client_fd = ::accept4(
+    listen_fd, (struct sockaddr *)&peer_addr, &peer_addr_len, SOCK_CLOEXEC);
   if (client_fd < 0) {
     if (errno == EBADF || errno == ENOTSOCK)
       throw ServerError(strerror(errno));