Merge pull request #2660 from stevenengler/epoll-ctl-check

 Remove unneeded workaround in `syscallhandler_epoll_ctl`

Author: stevenengler

src/main/host/descriptor/epoll.c

@@ -489,6 +489,23 @@ gint epoll_control(Epoll* epoll, gint operation, int fd, const Descriptor* descr
 
     switch (operation) {
         case EPOLL_CTL_ADD: {
+            /* Check if we're trying to add a file that's already been closed.
+             * Typically a file that is referenced in the descriptor table
+             * should never be a closed file, but Shadow's TCP sockets do close
+             * themselves even if there are still file handles (see
+             * `_tcp_endOfFileSignalled`), so we need to check this. */
+            Status status;
+            if (watchType == EWT_LEGACY_FILE) {
+                status = legacyfile_getStatus(watchObject.as_legacy_file);
+            } else if (watchType == EWT_GENERIC_FILE) {
+                status = file_getStatus(watchObject.as_file);
+            }
+            if (status & STATUS_FILE_CLOSED) {
+                warning("Attempted to add a closed file to epoll %p", epoll);
+                rv = -EBADF;
+                break;
+            }
+
             /* EEXIST op was EPOLL_CTL_ADD, and the supplied file descriptor
              * fd is already registered with this epoll instance. */
             if(watch) {

src/main/host/descriptor/tcp.c

@@ -2324,6 +2324,7 @@ static void _tcp_endOfFileSignalled(TCP* tcp, enum TCPFlags flags) {
 
     if((tcp->flags & TCPF_EOF_RD_SIGNALED) && (tcp->flags & TCPF_EOF_WR_SIGNALED)) {
         /* user can no longer access socket */
+        /* FIXME: a file should not be closed if there are still file handles (fds) to it */
         legacyfile_adjustStatus(&(tcp->super.super), STATUS_FILE_CLOSED, TRUE);
         legacyfile_adjustStatus(&(tcp->super.super), STATUS_FILE_ACTIVE, FALSE);
     }

src/main/host/syscall/epoll.c

@@ -106,12 +106,7 @@ SysCallReturn syscallhandler_epoll_ctl(SysCallHandler* sys,
     LegacyFile* legacyDescriptor = descriptor_asLegacyFile(descriptor);
 
     // Make sure the child is not closed only if it's a legacy file
-    // FIXME: for now we allow child fds to be closed on EPOLL_CTL_DEL operations,
-    // because libevent frequently closes before issuing the EPOLL_CTL_DEL op.
-    // Once #1101 is fixed, and we correctly clean up closed watch fds, then we can
-    // error out here on EPOLL_CTL_DEL ops too.
-    // See: https://github.com/shadow/shadow/issues/1101
-    if (legacyDescriptor != NULL && op != EPOLL_CTL_DEL) {
+    if (legacyDescriptor != NULL) {
         errorCode = _syscallhandler_validateLegacyFile(legacyDescriptor, DT_NONE);
 
         if (errorCode) {

src/main/host/syscall/protected.c

@@ -49,6 +49,12 @@ int _syscallhandler_validateLegacyFile(LegacyFile* descriptor, LegacyFileType ex
         Status status = legacyfile_getStatus(descriptor);
 
         if (status & STATUS_FILE_CLOSED) {
+            // A file that is referenced in the descriptor table should never
+            // be a closed file. File handles (fds) are handles to open files,
+            // so if we have a file handle to a closed file, then there's an
+            // error somewhere in Shadow. Shadow's TCP sockets do close
+            // themselves even if there are still file handles (see
+            // `_tcp_endOfFileSignalled`), so we can't make this a panic.
             warning("descriptor %p is closed", descriptor);
             return -EBADF;
         }