Part 2 of file descriptor race and deadlock corrections.

Adjust design of file descriptor table to eliminate races
with both opening and closing of file descriptor entries
and eliminates one class of deadlocks. One nice side effect
of this change in design should be better performance for
applications that open and close many file descriptors due
to reduced fd_table_lock contention and fd entry reuse.

- Add entry states to manage use of entry and eliminate
some closing races. fd entries are not deallocated upon
close() now.
- Call _thread_fd_table_init with one of five discreet
modes to properly initialize an entry and manage the
state transition to open.
- When closing an entry hold the entry spinlock locked
across the state transition and the _thread_sys_close
call to close another race.
- Introduce a new lock type FD_RDWR_CLOSE that transitions
either a closed entry or an open entry into closing state
and then waits for a RDWR lock so that the lock queue can
unwind normally. All subsequent fd lock attempts for that
entry are rejected with EBADF until the fd is fully closed,
or reopened by dup2(). Once a thread holds the FD_RDWR_LOCK
it is safe to close() it or dup2() on it.
- When a thread creates a new fd there is a window of time
when another thread could attempt to use the fd before the
creating thread has initialized the entry for it. This can
result in improper status_flags for the entry, so record
the entries init mode, detect when this has happened and
correct the status_flags when needed.

reviewed by marc@ & brad@, tested by several, okay brad@

1 files changed, 49 insertions, 8 deletions

diff --git a/lib/libpthread/uthread/uthread_closefrom.c b/lib/libpthread/uthread/uthread_closefrom.c
index d80c0cff21d..62176ec2d24 100644
--- a/lib/libpthread/uthread/uthread_closefrom.c
+++ b/lib/libpthread/uthread/uthread_closefrom.c
@@ -1,10 +1,11 @@
-/* $OpenBSD: uthread_closefrom.c,v 1.2 2006/09/22 19:04:33 kurt Exp $ */
+/* $OpenBSD: uthread_closefrom.c,v 1.3 2006/09/26 14:18:28 kurt Exp $ */
 
 /* PUBLIC DOMAIN: No Rights Reserved. Marco S Hyman <marc@snafu.org> */
 
 #include <sys/stat.h>
 
 #include <errno.h>
+#include <stdlib.h>
 #include <pthread.h>
 #include <unistd.h>
 
@@ -13,9 +14,10 @@
 int
 closefrom(int fd)
 {
-	int ret;
+	int ret = 0;
 	int safe_fd;
 	int lock_fd;
+	int *flags;
 
 	_thread_enter_cancellation_point();
 	
@@ -33,13 +35,52 @@ closefrom(int fd)
 		for (safe_fd++; fd < safe_fd; fd++)
 			close(fd);
 
-		/* Reset flags and clean up the fd table for fds to close */
-		for (lock_fd = fd; lock_fd < _thread_dtablesize; lock_fd++)
-			if (_thread_fd_table[lock_fd] != NULL)
-				_thread_fd_table_remove(lock_fd);
+		flags = calloc((size_t)_thread_dtablesize, sizeof *flags);
+		if (flags == NULL) {
+			/* use calloc errno */
+			ret = -1;
+		} else {
+			/* Lock and record all fd entries */
+			for (lock_fd = fd; lock_fd < _thread_dtablesize; lock_fd++) {
+				if (_thread_fd_table[lock_fd] != NULL &&
+			   	 _thread_fd_table[lock_fd]->state != FD_ENTRY_CLOSED) {
+					ret = _FD_LOCK(lock_fd, FD_RDWR_CLOSE, NULL);
+					if (ret != -1)
+						flags[lock_fd] = 1;
+					else
+						break;
+				}
+			}
 
-		/* Now let the system do its thing */
-		ret = _thread_sys_closefrom(fd);
+			if (ret != -1) {
+				/*
+				 * Close the entries and reset the non-bocking
+				 * flag when needed.
+				 */
+				for (lock_fd = fd; lock_fd < _thread_dtablesize; lock_fd++) {
+					if (flags[lock_fd] != NULL) {
+						_thread_fd_entry_close(lock_fd);
+					}
+				}
+				/*
+				 * Now let the system do its thing. It is not practical
+				 * to try to prevent races with other threads that can
+				 * create new file descriptors. We just have to assume
+				 * the application is well behaved when using closefrom.
+				 */
+				ret = _thread_sys_closefrom(fd);
+			}
+
+			/*
+			 * Unlock any locked entries.
+			 */
+			for (lock_fd = fd; lock_fd < _thread_dtablesize; lock_fd++) {
+				if (flags[lock_fd] != NULL) {
+					_FD_UNLOCK(lock_fd, FD_RDWR_CLOSE);
+				}
+			}
+			free(flags);
+		}
 	}
 
 	_thread_leave_cancellation_point();