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@

19 files changed, 345 insertions, 137 deletions

diff --git a/lib/libc/include/thread_private.h b/lib/libc/include/thread_private.h
index 5fbbf592a41..c5acc4b5e43 100644
--- a/lib/libc/include/thread_private.h
+++ b/lib/libc/include/thread_private.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: thread_private.h,v 1.18 2006/02/22 07:16:31 otto Exp $ */
+/* $OpenBSD: thread_private.h,v 1.19 2006/09/26 14:18:28 kurt Exp $ */
 
 /* PUBLIC DOMAIN: No Rights Reserved. Marco S Hyman <marc@snafu.org> */
 
@@ -90,6 +90,7 @@ extern void *__THREAD_NAME(serv_mutex);
 #define FD_READ		0x1
 #define FD_WRITE	0x2
 #define FD_RDWR		(FD_READ | FD_WRITE)
+#define FD_RDWR_CLOSE	(FD_RDWR | 0x4)
 
 struct timespec;
 int	_thread_fd_lock(int, int, struct timespec *);
diff --git a/lib/libpthread/uthread/pthread_private.h b/lib/libpthread/uthread/pthread_private.h
index 35c927e31fa..43e55c2b001 100644
--- a/lib/libpthread/uthread/pthread_private.h
+++ b/lib/libpthread/uthread/pthread_private.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pthread_private.h,v 1.56 2006/09/22 19:04:33 kurt Exp $	*/
+/*	$OpenBSD: pthread_private.h,v 1.57 2006/09/26 14:18:28 kurt Exp $	*/
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>.
  * All rights reserved.
@@ -506,6 +506,26 @@ struct fs_flags {
 };
 
 /*
+ * fd_table_entry states
+ */
+enum fd_entry_state {
+	FD_ENTRY_OPEN,
+	FD_ENTRY_CLOSING,
+	FD_ENTRY_CLOSED
+};
+
+/*
+ * Defines for _thread_fd_table_init init_mode
+ */
+enum fd_entry_mode {
+	FD_INIT_UNKNOWN,	/* inherited or not created by pthreads wrapper */
+	FD_INIT_NEW,		/* new fd opened by pthreads */
+	FD_INIT_BLOCKING,	/* new user blocking fd opened by pthreads */
+	FD_INIT_DUP,		/* new fd with passed flags */
+	FD_INIT_DUP2,		/* replace status_flags and open */
+};
+
+/*
  * File descriptor table structure.
  */
 struct fd_table_entry {
@@ -527,6 +547,8 @@ struct fd_table_entry {
 	int			r_lockcount;	/* Count for FILE read locks. */
 	int			w_lockcount;	/* Count for FILE write locks.*/
 	struct fs_flags		*status_flags;	/* Shared file status flags.  */
+	enum fd_entry_state	state;		/* Open, closing, or closed.  */
+	enum fd_entry_mode	init_mode;	/* The mode used for init.    */
 };
 
 struct pthread_poll_data {
@@ -1132,9 +1154,8 @@ void    _thread_start_sig_handler(void);
 void	_thread_seterrno(pthread_t,int);
 void	_thread_fs_flags_replace(int, struct fs_flags *);
 void	_thread_fd_init(void);
-int     _thread_fd_table_init(int, struct fs_flags *);
-int     _thread_fd_table_dup(int, int);
-void	_thread_fd_table_remove(int);
+int     _thread_fd_table_init(int, enum fd_entry_mode, struct fs_flags *);
+void	_thread_fd_entry_close(int);
 void	_thread_fd_unlock_owned(pthread_t);
 void	_thread_fd_unlock_thread(struct pthread	*, int, int);
 pthread_addr_t _thread_gc(pthread_addr_t);
diff --git a/lib/libpthread/uthread/uthread_accept.c b/lib/libpthread/uthread/uthread_accept.c
index a84312ba56f..caaa398dc8a 100644
--- a/lib/libpthread/uthread/uthread_accept.c
+++ b/lib/libpthread/uthread/uthread_accept.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_accept.c,v 1.9 2006/09/22 19:04:33 kurt Exp $	*/
+/*	$OpenBSD: uthread_accept.c,v 1.10 2006/09/26 14:18:28 kurt Exp $	*/
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>
  * All rights reserved.
@@ -46,6 +46,8 @@ accept(int fd, struct sockaddr * name, socklen_t *namelen)
 {
 	struct pthread	*curthread = _get_curthread();
 	int             ret;
+	int		newfd;
+	enum fd_entry_mode init_mode;
 
 	/* This is a cancellation point: */
 	_thread_enter_cancellation_point();
@@ -88,17 +90,23 @@ accept(int fd, struct sockaddr * name, socklen_t *namelen)
 
 		/*
 		 * If no errors initialize the file descriptor table
-		 * for the new socket.   Turn on blocking mode in the
-		 * child if it is on in the parent.   This is done
-		 * as _thread_fd_table_init *may* have turned the flag on.
+		 * for the new socket. If the client's view of the
+		 * status_flags for fd is blocking, then force newfd
+		 * to be viewed as blocking too.
 		 */
 		if (ret != -1) {
-			if (_thread_fd_table_init(ret, NULL) != 0) {
-				/* Quietly close the socket: */
+			newfd = ret;
+
+			if ((_thread_fd_table[fd]->status_flags->flags & O_NONBLOCK) == 0)
+				init_mode = FD_INIT_BLOCKING;
+			else
+				init_mode = FD_INIT_NEW;
+			if((ret = _thread_fd_table_init(newfd, init_mode, NULL)) != -1)
+				ret = newfd;
+			else {
+				/* quitely close the fd */
 				_thread_sys_close(ret);
-				ret = -1;
-			} else if ((_thread_fd_table[fd]->status_flags->flags & O_NONBLOCK) == 0)
-				_thread_fd_table[ret]->status_flags->flags &= ~O_NONBLOCK;
+			}
 		}
 
 		/* Unlock the file descriptor: */
diff --git a/lib/libpthread/uthread/uthread_close.c b/lib/libpthread/uthread/uthread_close.c
index fa45be87e68..05057dd026f 100644
--- a/lib/libpthread/uthread/uthread_close.c
+++ b/lib/libpthread/uthread/uthread_close.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_close.c,v 1.12 2006/09/22 19:04:33 kurt Exp $	*/
+/*	$OpenBSD: uthread_close.c,v 1.13 2006/09/26 14:18:28 kurt Exp $	*/
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>
  * All rights reserved.
@@ -51,15 +51,22 @@ close(int fd)
 	    (fd == _thread_kern_pipe[0]) || (fd == _thread_kern_pipe[1])) {
 		errno = EBADF;
 		ret = -1;
-	} else if (_thread_fd_table[fd] == NULL)
-		/* unknown to thread kernel, let system handle the close */
-		ret = _thread_sys_close(fd);
-	else if ((ret = _FD_LOCK(fd, FD_RDWR, NULL)) == 0) {
-		/* XXX: Assumes well behaved threads. */
-		/* XXX: Defer real close to avoid race condition */
-		_thread_fd_table_remove(fd);
+	} else if ((ret = _FD_LOCK(fd, FD_RDWR_CLOSE, NULL)) != -1) {
+		/*
+		 * We need to hold the entry spinlock till after
+		 * _thread_sys_close() to stop races caused by the
+		 * fd state transition.
+		 */
+		_SPINLOCK(&_thread_fd_table[fd]->lock);
+
+		_thread_fd_entry_close(fd);
+
 		/* Close the file descriptor: */
 		ret = _thread_sys_close(fd);
+
+		_SPINUNLOCK(&_thread_fd_table[fd]->lock);
+
+		_FD_UNLOCK(fd, FD_RDWR_CLOSE);
 	}
 
 	/* No longer in a cancellation point: */
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();
diff --git a/lib/libpthread/uthread/uthread_dup.c b/lib/libpthread/uthread/uthread_dup.c
index 7b7d01c3069..af0fa71da6d 100644
--- a/lib/libpthread/uthread/uthread_dup.c
+++ b/lib/libpthread/uthread/uthread_dup.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: uthread_dup.c,v 1.6 2006/09/22 19:04:33 kurt Exp $ */
+/* $OpenBSD: uthread_dup.c,v 1.7 2006/09/26 14:18:28 kurt Exp $ */
 /* PUBLIC DOMAIN <marc@snafu.org> */
 
 #include <unistd.h>
@@ -15,7 +15,7 @@ dup(int fd)
 	if (ret == 0) {
 		ret = _thread_sys_dup(fd);
 		if (ret != -1) {
-			if (_thread_fd_table_init(ret,
+			if (_thread_fd_table_init(ret, FD_INIT_DUP,
 			    _thread_fd_table[fd]->status_flags) == -1) {
 				_thread_sys_close(ret);
 				ret = -1;
diff --git a/lib/libpthread/uthread/uthread_dup2.c b/lib/libpthread/uthread/uthread_dup2.c
index d8ef28d729d..ea1b88c7c5e 100644
--- a/lib/libpthread/uthread/uthread_dup2.c
+++ b/lib/libpthread/uthread/uthread_dup2.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: uthread_dup2.c,v 1.8 2006/09/22 19:04:33 kurt Exp $ */
+/* $OpenBSD: uthread_dup2.c,v 1.9 2006/09/26 14:18:28 kurt Exp $ */
 /* PUBLIC DOMAIN <marc@snafu.org> */
 
 #include <errno.h>
@@ -12,25 +12,22 @@ dup2(int fd, int newfd)
 {
 	int	ret;
 	int	saved_errno;
-	int	newfd_opened;
 
 	if (newfd >= 0 && newfd < _thread_dtablesize &&
 	    newfd != _thread_kern_pipe[0] && newfd != _thread_kern_pipe[1]) {
 		ret = _FD_LOCK(fd, FD_RDWR, NULL);
 		if (ret == 0) {
-			newfd_opened = _thread_fd_table[newfd] != NULL;
-			if (!newfd_opened ||
-			    (ret = _FD_LOCK(newfd, FD_RDWR, NULL)) == 0) {
+			if ((ret = _FD_LOCK(newfd, FD_RDWR_CLOSE, NULL)) == 0) {
 				/*
 				 * If newfd was open then drop reference
 				 * and reset flags if needed.
 				 */
-				if (newfd_opened)
-					_thread_fs_flags_replace(newfd, NULL);
+				_thread_fs_flags_replace(newfd, NULL);
 				ret = _thread_sys_dup2(fd, newfd);
 				if (ret != -1)
-					ret = _thread_fd_table_init(newfd,
+					ret = _thread_fd_table_init(newfd, FD_INIT_DUP2,
 					    _thread_fd_table[fd]->status_flags);
+
 				/*
 				 * If the dup2 or the _thread_fd_table_init
 				 * failed we've already removed the status
@@ -40,15 +37,15 @@ dup2(int fd, int newfd)
 				 */
 				if (ret == -1) {
 					saved_errno = errno;
+					_SPINLOCK(&_thread_fd_table[newfd]->lock);
 
-					if (newfd_opened)
-						_thread_fd_table_remove(newfd);
+					_thread_fd_entry_close(newfd);
 					_thread_sys_close(newfd);
 					
+					_SPINUNLOCK(&_thread_fd_table[newfd]->lock);
 					errno = saved_errno ;
-				} else if (newfd_opened) {
-					_FD_UNLOCK(newfd, FD_RDWR);
 				}
+				_FD_UNLOCK(newfd, FD_RDWR_CLOSE);
 			}
 			_FD_UNLOCK(fd, FD_RDWR);
 		}
diff --git a/lib/libpthread/uthread/uthread_execve.c b/lib/libpthread/uthread/uthread_execve.c
index 7769d3e6685..bc45ab1d983 100644
--- a/lib/libpthread/uthread/uthread_execve.c
+++ b/lib/libpthread/uthread/uthread_execve.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_execve.c,v 1.8 2006/09/22 19:04:33 kurt Exp $	*/
+/*	$OpenBSD: uthread_execve.c,v 1.9 2006/09/26 14:18:28 kurt Exp $	*/
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>
  * All rights reserved.
@@ -68,7 +68,8 @@ execve(const char *name, char *const * argv, char *const * envp)
 	for (i = 0; i < _thread_dtablesize; i++) {
 		/* Check if this file descriptor is in use: */
 		if (_thread_fd_table[i] != NULL &&
-			!(_thread_fd_table[i]->status_flags->flags & O_NONBLOCK)) {
+		    _thread_fd_table[i]->status_flags != NULL &&
+		    !(_thread_fd_table[i]->status_flags->flags & O_NONBLOCK)) {
 			/* Skip if the close-on-exec flag is set */
 			flags = _thread_sys_fcntl(i, F_GETFD, NULL);
 			if ((flags & FD_CLOEXEC) != 0)
diff --git a/lib/libpthread/uthread/uthread_exit.c b/lib/libpthread/uthread/uthread_exit.c
index 94fc1699c37..4836123b2b1 100644
--- a/lib/libpthread/uthread/uthread_exit.c
+++ b/lib/libpthread/uthread/uthread_exit.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_exit.c,v 1.18 2006/09/22 19:04:33 kurt Exp $	*/
+/*	$OpenBSD: uthread_exit.c,v 1.19 2006/09/26 14:18:28 kurt Exp $	*/
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>
  * All rights reserved.
@@ -67,7 +67,8 @@ _exit(int status)
 	for (i = 0; i < _thread_dtablesize; i++) {
 		/* Check if this file descriptor is in use: */
 		if (_thread_fd_table[i] != NULL &&
-			!(_thread_fd_table[i]->status_flags->flags & O_NONBLOCK)) {
+		    _thread_fd_table[i]->status_flags != NULL &&
+		    !(_thread_fd_table[i]->status_flags->flags & O_NONBLOCK)) {
 			/* Get the current flags: */
 			flags = _thread_sys_fcntl(i, F_GETFL, NULL);
 			/* Clear the nonblocking file descriptor flag: */
diff --git a/lib/libpthread/uthread/uthread_fcntl.c b/lib/libpthread/uthread/uthread_fcntl.c
index 0b772778ece..bf9f70e839c 100644
--- a/lib/libpthread/uthread/uthread_fcntl.c
+++ b/lib/libpthread/uthread/uthread_fcntl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_fcntl.c,v 1.8 2006/09/22 19:04:33 kurt Exp $	*/
+/*	$OpenBSD: uthread_fcntl.c,v 1.9 2006/09/26 14:18:28 kurt Exp $	*/
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>
  * All rights reserved.
@@ -70,7 +70,7 @@ fcntl(int fd, int cmd,...)
 			if ((ret = _thread_sys_fcntl(fd, cmd, oldfd)) < 0) {
 			}
 			/* Initialise the file descriptor table entry: */
-			else if (_thread_fd_table_init(ret,
+			else if (_thread_fd_table_init(ret, FD_INIT_DUP,
 			    _thread_fd_table[fd]->status_flags) == -1) {
 				/* Quietly close the file: */
 				_thread_sys_close(ret);
diff --git a/lib/libpthread/uthread/uthread_fd.c b/lib/libpthread/uthread/uthread_fd.c
index 666b94ab20d..dfdba6fb50f 100644
--- a/lib/libpthread/uthread/uthread_fd.c
+++ b/lib/libpthread/uthread/uthread_fd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_fd.c,v 1.24 2006/09/23 12:25:58 kurt Exp $	*/
+/*	$OpenBSD: uthread_fd.c,v 1.25 2006/09/26 14:18:28 kurt Exp $	*/
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>
  * All rights reserved.
@@ -96,7 +96,7 @@ _thread_fs_flags_init(struct fs_flags *status_flags, int fd)
 /*
  * If existing entry's status_flags don't match new one,
  * then replace the current status flags with the new one.
- * It is assumed the entry is locked with a FD_RDWR
+ * It is assumed the entry is locked with a FD_RDWR_CLOSE
  * lock when this function is called.
  */
 void
@@ -177,6 +177,8 @@ _thread_fd_entry(void)
 		_SPINLOCK_INIT(&entry->lock);
 		TAILQ_INIT(&entry->r_queue);
 		TAILQ_INIT(&entry->w_queue);
+		entry->state = FD_ENTRY_CLOSED;
+		entry->init_mode = FD_INIT_UNKNOWN;
 	}
 	return entry;
 }
@@ -233,6 +235,8 @@ _thread_fd_init(void)
 					if (entry2 != NULL) {
 						status_flags->refcnt += 1;
 						entry2->status_flags = status_flags;
+						entry2->state = FD_ENTRY_OPEN;
+						entry2->init_mode = FD_INIT_DUP2;
 						_thread_fd_table[fd2] = entry2;
 					} else
 						PANIC("Cannot allocate memory for flags table");
@@ -243,6 +247,8 @@ _thread_fd_init(void)
 				status_flags->refcnt += 1;
 				status_flags->flags = flags[fd];
 				entry1->status_flags = status_flags;
+				entry1->state = FD_ENTRY_OPEN;
+				entry1->init_mode = FD_INIT_DUP2;
 				_thread_fd_table[fd] = entry1;
 				flags[fd] |= O_NONBLOCK;
 			} else {
@@ -276,11 +282,12 @@ _thread_fd_init(void)
  * calls.
  */
 int
-_thread_fd_table_init(int fd, struct fs_flags *status_flags)
+_thread_fd_table_init(int fd, enum fd_entry_mode init_mode, struct fs_flags *status_flags)
 {
 	int	ret = 0;
+	int	saved_errno;
 	struct fd_table_entry *entry;
-	struct fs_flags *new_status_flags = NULL;
+	struct fs_flags *new_status_flags;
 
 	if (fd < 0 || fd >= _thread_dtablesize) {
 		/*
@@ -295,62 +302,172 @@ _thread_fd_table_init(int fd, struct fs_flags *status_flags)
 		/* First time for this fd, build an entry */
 		entry = _thread_fd_entry();
 		if (entry == NULL) {
-			errno = ENOMEM;
+			/* use _thread_fd_entry errno */
 			ret = -1;
 		} else {
-			if (status_flags == NULL) {
+			/* Lock the file descriptor table: */
+			_SPINLOCK(&fd_table_lock);
+
+			/*
+			 * Check if another thread allocated the
+			 * file descriptor entry while this thread
+			 * was doing the same thing. The table wasn't
+			 * kept locked during this operation because
+			 * it has the potential to recurse.
+			 */
+			if (_thread_fd_table[fd] == NULL) {
+				/* This thread wins: */
+				_thread_fd_table[fd] = entry;
+				entry = NULL;
+			}
+
+			/* Unlock the file descriptor table: */
+			_SPINUNLOCK(&fd_table_lock);
+
+			/*
+			 * If another thread initialized the table entry
+			 * throw the new entry away.
+			 */
+			if (entry != NULL)
+				free(entry);
+		}
+	}
+
+	if (ret == 0) {
+		entry = _thread_fd_table[fd];
+		_SPINLOCK(&entry->lock);
+		switch (init_mode) {
+		case FD_INIT_UNKNOWN:
+			/*
+			 * If the entry is closed, try to open it
+			 * anyway since we may have inherited it or
+			 * it may have been created by an unwrapped
+			 * call such as openpty(3). Since we allow
+			 * FD_RDWR_CLOSE locks on closed entries,
+			 * we ignore EBADF status flags errors and
+			 * return a closed entry. If the entry is
+			 * not closed then there's nothing to do.
+			 */
+			if (entry->state == FD_ENTRY_CLOSED) {
 				new_status_flags = _thread_fs_flags_entry();
-				if (new_status_flags == NULL)
+				if (new_status_flags == NULL) {
+					/* use _thread_fs_flags_entry errno */
 					ret = -1;
-				else
+				} else {
+					saved_errno = errno;
 					ret = _thread_fs_flags_init(new_status_flags, fd);
-			}
-			if (ret == 0) {
-				/* Lock the file descriptor table: */
-				_SPINLOCK(&fd_table_lock);
-
-				/*
-				 * Check if another thread allocated the
-				 * file descriptor entry while this thread
-				 * was doing the same thing. The table wasn't
-				 * kept locked during this operation because
-				 * it has the potential to recurse.
-				 */
-				if (_thread_fd_table[fd] == NULL) {
-					if (status_flags != NULL) {
-						_SPINLOCK(&status_flags->lock);
-						status_flags->refcnt += 1;
-						_SPINUNLOCK(&status_flags->lock);
-						entry->status_flags = status_flags;
-					} else {
+					if (ret == 0) {
+						errno = saved_errno;
 						new_status_flags->refcnt = 1;
 						entry->status_flags = new_status_flags;
+						new_status_flags = NULL;
+						entry->state = FD_ENTRY_OPEN;
+						entry->init_mode = init_mode;
+					} else if (errno == EBADF) {
+						errno = saved_errno;
+						ret = 0;
 					}
-					/* This thread wins: */
-					_thread_fd_table[fd] = entry;
-					entry = NULL;
+				}
+				/* if flags init failed free new flags */
+				if (new_status_flags != NULL)
+					free(new_status_flags);
+			}
+			break;
+		case FD_INIT_NEW:
+			/*
+			 * If the entry was initialized and opened
+			 * by another thread (i.e. FD_INIT_DUP2 or
+			 * FD_INIT_UNKNOWN), the status flags will
+			 * be correct.
+			 */
+			if (entry->state == FD_ENTRY_CLOSED) {
+				new_status_flags = _thread_fs_flags_entry();
+				if (new_status_flags == NULL) {
+					/* use _thread_fs_flags_entry errno */
+					ret = -1;
+				} else {
+					ret = _thread_fs_flags_init(new_status_flags, fd);
+				}
+				if (ret == 0) {
+					new_status_flags->refcnt = 1;
+					entry->status_flags = new_status_flags;
 					new_status_flags = NULL;
+					entry->state = FD_ENTRY_OPEN;
+					entry->init_mode = init_mode;
 				}
-
-				/* Unlock the file descriptor table: */
-				_SPINUNLOCK(&fd_table_lock);
+				/* if flags init failed free new flags */
+				if (new_status_flags != NULL)
+					free(new_status_flags);
 			}
-
+			break;
+		case FD_INIT_BLOCKING:
 			/*
-			 * If there was an error in getting the flags for
-			 * the file or if another thread initialized the
-			 * table entry throw this entry and new_status_flags
-			 * away.
+			 * If the entry was initialized and opened
+			 * by another thread with FD_INIT_DUP2, the
+			 * status flags will be correct. However,
+			 * if FD_INIT_UNKNOWN raced in before us
+			 * it means the app is not well behaved and
+			 * tried to use the fd before it was returned
+			 * to the client.
+			 */
+			if (entry->state == FD_ENTRY_CLOSED) {
+				new_status_flags = _thread_fs_flags_entry();
+				if (new_status_flags == NULL) {
+					/* use _thread_fs_flags_entry errno */
+					ret = -1;
+				} else {
+					ret = _thread_fs_flags_init(new_status_flags, fd);
+				}
+				if (ret == 0) {
+					/* set user's view of status flags to blocking */
+					new_status_flags->flags &= ~O_NONBLOCK;
+					new_status_flags->refcnt = 1;
+					entry->status_flags = new_status_flags;
+					new_status_flags = NULL;
+					entry->state = FD_ENTRY_OPEN;
+					entry->init_mode = init_mode;
+				}
+				/* if flags init failed free new flags */
+				if (new_status_flags != NULL)
+					free(new_status_flags);
+			} else if (entry->state == FD_ENTRY_OPEN &&
+			    entry->init_mode == FD_INIT_UNKNOWN) {
+				entry->status_flags->flags &= ~O_NONBLOCK;
+			}
+			break;
+		case FD_INIT_DUP:
+			/*
+			 * If the entry was initialized and opened
+			 * by another thread with FD_INIT_DUP2 then
+			 * keep it. However, if FD_INIT_UNKNOWN raced
+			 * in before us it means the app is not well
+			 * behaved and tried to use the fd before it
+			 * was returned to the client.
+			 */
+			if (entry->state == FD_ENTRY_CLOSED) {
+				_thread_fs_flags_replace(fd, status_flags);
+				entry->state = FD_ENTRY_OPEN;
+				entry->init_mode = init_mode;
+			} else if (entry->state == FD_ENTRY_OPEN &&
+			    entry->init_mode == FD_INIT_UNKNOWN) {
+				_thread_fs_flags_replace(fd, status_flags);
+			}
+			break;
+		case FD_INIT_DUP2:
+			/*
+			 * This is only called when FD_RDWR_CLOSE
+			 * is held and in state FD_ENTRY_CLOSING.
+			 * Just replace flags and open entry.
+			 * FD_INIT_UNKNOWN can't race in since we
+			 * are in state FD_ENTRY_CLOSING before
+			 * the _thread_sys_dup2 happens.
 			 */
-			if (entry != NULL)
-				free(entry);
-
-			if (new_status_flags != NULL)
-				free(new_status_flags);
-		}
-	} else {
-		if (status_flags != NULL)
 			_thread_fs_flags_replace(fd, status_flags);
+			entry->state = FD_ENTRY_OPEN;
+			entry->init_mode = init_mode;
+			break;
+		}
+		_SPINUNLOCK(&entry->lock);
 	}
 
 	/* Return the completion status: */
@@ -358,19 +475,15 @@ _thread_fd_table_init(int fd, struct fs_flags *status_flags)
 }
 
 /*
- * Remove an fd entry from the table and replace its status flags
- * with NULL. The entry is assummed to be locked with a RDWR lock.
+ * Close an fd entry. Replace existing status flags
+ * with NULL. The entry is assummed to be locked with
+ * a FD_RDWR_CLOSE lock and in state FD_ENTRY_CLOSING.
  */
 void
-_thread_fd_table_remove(int fd)
+_thread_fd_entry_close(int fd)
 {
-	_SPINLOCK(&fd_table_lock);
-
 	_thread_fs_flags_replace(fd, NULL);
-	free(_thread_fd_table[fd]);
-	_thread_fd_table[fd] = NULL;
-
-	_SPINUNLOCK(&fd_table_lock);
+	_thread_fd_table[fd]->state = FD_ENTRY_CLOSED;
 }
 
 /*
@@ -380,14 +493,12 @@ void
 _thread_fd_unlock_thread(struct pthread	*thread, int fd, int lock_type)
 {
 	struct fd_table_entry *entry;
-	int	ret;
 
 	/*
-	 * Check that the file descriptor table is initialised for this
-	 * entry: 
-	 */
-	ret = _thread_fd_table_init(fd, NULL);
-	if (ret == 0) {
+	 * If file descriptor is out of range or uninitialized,
+	 * do nothing.
+	 */ 
+	if (fd >= 0 && fd < _thread_dtablesize && _thread_fd_table[fd] != NULL) {
 		entry = _thread_fd_table[fd];
 
 		/*
@@ -405,7 +516,7 @@ _thread_fd_unlock_thread(struct pthread	*thread, int fd, int lock_type)
 
 		/* Check if the running thread owns the read lock: */
 		if (entry->r_owner == thread &&
-		    (lock_type == FD_READ || lock_type == FD_RDWR)) {
+		    (lock_type & FD_READ)) {
 			/*
 			 * Decrement the read lock count for the
 			 * running thread: 
@@ -441,7 +552,7 @@ _thread_fd_unlock_thread(struct pthread	*thread, int fd, int lock_type)
 		}
 		/* Check if the running thread owns the write lock: */
 		if (entry->w_owner == thread &&
-		    (lock_type == FD_WRITE || lock_type == FD_RDWR)) {
+		    (lock_type & FD_WRITE)) {
 			/*
 			 * Decrement the write lock count for the
 			 * running thread: 
@@ -485,9 +596,6 @@ _thread_fd_unlock_thread(struct pthread	*thread, int fd, int lock_type)
 		 */
 		_thread_kern_sig_undefer();
 	}
-
-	/* Nothing to return. */
-	return;
 }
 
 /*
@@ -545,7 +653,7 @@ _thread_fd_lock(int fd, int lock_type, struct timespec * timeout)
 	 * Check that the file descriptor table is initialised for this
 	 * entry: 
 	 */
-	ret = _thread_fd_table_init(fd, NULL);
+	ret = _thread_fd_table_init(fd, FD_INIT_UNKNOWN, NULL);
 	if (ret == 0) {
 		entry = _thread_fd_table[fd];
 
@@ -556,8 +664,20 @@ _thread_fd_lock(int fd, int lock_type, struct timespec * timeout)
 		 */
 		_SPINLOCK(&entry->lock);
 
+		/* reject all new locks on entries that are closing */
+		if (entry->state == FD_ENTRY_CLOSING) {
+			ret = -1;
+			errno = EBADF;
+		} else if (lock_type == FD_RDWR_CLOSE) {
+			/* allow closing locks on open and closed entries */
+			entry->state = FD_ENTRY_CLOSING;
+		} else if (entry->state == FD_ENTRY_CLOSED) {
+			ret = -1;
+			errno = EBADF;
+		}
+
 		/* Handle read locks */
-		if (lock_type == FD_READ || lock_type == FD_RDWR) {
+		if (ret == 0 && (lock_type & FD_READ)) {
 			/*
 			 * Enter a loop to wait for the file descriptor to be
 			 * locked    for read for the current thread: 
@@ -631,7 +751,7 @@ _thread_fd_lock(int fd, int lock_type, struct timespec * timeout)
 		}
 
 		/* Handle write locks */
-		if (lock_type == FD_WRITE || lock_type == FD_RDWR) {
+		if ( ret == 0 && (lock_type & FD_WRITE)) {
 			/*
 			 * Enter a loop to wait for the file descriptor to be
 			 * locked for write for the current thread: 
diff --git a/lib/libpthread/uthread/uthread_fork.c b/lib/libpthread/uthread/uthread_fork.c
index 3c51d6cf305..f3a0230e632 100644
--- a/lib/libpthread/uthread/uthread_fork.c
+++ b/lib/libpthread/uthread/uthread_fork.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_fork.c,v 1.13 2006/09/22 19:04:33 kurt Exp $	*/
+/*	$OpenBSD: uthread_fork.c,v 1.14 2006/09/26 14:18:28 kurt Exp $	*/
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>
  * All rights reserved.
@@ -187,6 +187,14 @@ fork(void)
 					_thread_fd_table[i]->w_lockcount = 0;
 					if (_thread_fd_table[i]->status_flags != NULL)
 						_SPINLOCK_INIT(&_thread_fd_table[i]->status_flags->lock);
+					/*
+					 * If a fd was in the process of closing
+					 * then finish closing it.
+					 */
+					if (_thread_fd_table[i]->state == FD_ENTRY_CLOSING) {
+						 _thread_fd_entry_close(i);
+						 _thread_sys_close(i);
+					}
 
 					/* Initialise the read/write queues: */
 					TAILQ_INIT(&_thread_fd_table[i]->r_queue);
diff --git a/lib/libpthread/uthread/uthread_info_openbsd.c b/lib/libpthread/uthread/uthread_info_openbsd.c
index 1723d80b807..70735efe97b 100644
--- a/lib/libpthread/uthread/uthread_info_openbsd.c
+++ b/lib/libpthread/uthread/uthread_info_openbsd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_info_openbsd.c,v 1.10 2006/04/09 02:57:41 krw Exp $	*/
+/*	$OpenBSD: uthread_info_openbsd.c,v 1.11 2006/09/26 14:18:28 kurt Exp $	*/
 
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>
@@ -185,7 +185,8 @@ _thread_dump_entry(pthread_t pthread, int fd, int verbose)
 		    pthread->data.fd.branch);
 		_thread_sys_write(fd, s, strlen(s));
 		s[0] = 0;
-		if (_thread_fd_table[pthread->data.fd.fd])
+		if (_thread_fd_table[pthread->data.fd.fd] &&
+		    _thread_fd_table[pthread->data.fd.fd]->state != FD_ENTRY_CLOSED)
 		    snprintf(s, sizeof(s), "%s owner %pr/%pw\n",
 			     info_lead,
 			     _thread_fd_table[pthread->data.fd.fd]->r_owner,
@@ -361,7 +362,8 @@ _thread_dump_info(void)
 		 */
 		char rcode[22], wcode[22];
 
-		if (_thread_fd_table[i] != NULL) {
+		if (_thread_fd_table[i] != NULL &&
+		    _thread_fd_table[i]->state != FD_ENTRY_CLOSED) {
 
 			/* Find the reader's last file:line: */
 			if (_thread_fd_table[i]->r_owner)
diff --git a/lib/libpthread/uthread/uthread_kqueue.c b/lib/libpthread/uthread/uthread_kqueue.c
index 638d9fe445a..e9c751b9807 100644
--- a/lib/libpthread/uthread/uthread_kqueue.c
+++ b/lib/libpthread/uthread/uthread_kqueue.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_kqueue.c,v 1.2 2006/09/22 19:04:33 kurt Exp $	*/
+/*	$OpenBSD: uthread_kqueue.c,v 1.3 2006/09/26 14:18:28 kurt Exp $	*/
 
 /*
  * Copyright (c) 2003 Mark Peek <mp@freebsd.org>
@@ -46,7 +46,7 @@ kqueue(void)
 		/* Error creating socket. */
 
 	/* Initialise the entry in the file descriptor table: */
-	} else if (_thread_fd_table_init(fd, NULL) != 0) {
+	} else if (_thread_fd_table_init(fd, FD_INIT_NEW, NULL) != 0) {
 		_thread_sys_close(fd);
 		fd = -1;
 	}
diff --git a/lib/libpthread/uthread/uthread_open.c b/lib/libpthread/uthread/uthread_open.c
index aa2d3dd9b56..f49bfe6d22b 100644
--- a/lib/libpthread/uthread/uthread_open.c
+++ b/lib/libpthread/uthread/uthread_open.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_open.c,v 1.7 2006/09/22 19:04:33 kurt Exp $	*/
+/*	$OpenBSD: uthread_open.c,v 1.8 2006/09/26 14:18:28 kurt Exp $	*/
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>
  * All rights reserved.
@@ -63,7 +63,7 @@ open(const char *path, int flags,...)
 	if ((fd = _thread_sys_open(path, flags, mode)) < 0) {
 	}
 	/* Initialise the file descriptor table entry: */
-	else if (_thread_fd_table_init(fd, NULL) != 0) {
+	else if (_thread_fd_table_init(fd, FD_INIT_NEW, NULL) != 0) {
 		/* Quietly close the file: */
 		_thread_sys_close(fd);
 
diff --git a/lib/libpthread/uthread/uthread_pipe.c b/lib/libpthread/uthread/uthread_pipe.c
index ca798183769..9636e59700b 100644
--- a/lib/libpthread/uthread/uthread_pipe.c
+++ b/lib/libpthread/uthread/uthread_pipe.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_pipe.c,v 1.4 2006/09/22 19:04:33 kurt Exp $	*/
+/*	$OpenBSD: uthread_pipe.c,v 1.5 2006/09/26 14:18:28 kurt Exp $	*/
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>
  * All rights reserved.
@@ -43,8 +43,8 @@ pipe(int fds[2])
 {
 	int             ret;
 	if ((ret = _thread_sys_pipe(fds)) >= 0) {
-		if (_thread_fd_table_init(fds[0], NULL) != 0 ||
-		    _thread_fd_table_init(fds[1], NULL) != 0) {
+		if (_thread_fd_table_init(fds[0], FD_INIT_NEW, NULL) != 0 ||
+		    _thread_fd_table_init(fds[1], FD_INIT_NEW, NULL) != 0) {
 			_thread_sys_close(fds[0]);
 			_thread_sys_close(fds[1]);
 			ret = -1;
diff --git a/lib/libpthread/uthread/uthread_sig.c b/lib/libpthread/uthread/uthread_sig.c
index 7f4d9dd082c..73b2cdfd8ae 100644
--- a/lib/libpthread/uthread/uthread_sig.c
+++ b/lib/libpthread/uthread/uthread_sig.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_sig.c,v 1.21 2006/09/22 19:04:33 kurt Exp $	*/
+/*	$OpenBSD: uthread_sig.c,v 1.22 2006/09/26 14:18:28 kurt Exp $	*/
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>
  * All rights reserved.
@@ -185,7 +185,8 @@ _thread_sig_handle(int sig, struct sigcontext * scp)
 			 * set some of them to block. Sigh.
 			 */
 			for (i = 0; i < _thread_dtablesize; i++)
-				if (_thread_fd_table[i] != NULL)
+				if (_thread_fd_table[i] != NULL &&
+				    _thread_fd_table[i]->status_flags != NULL)
 					_thread_sys_fcntl(i, F_SETFL,
 					    _thread_fd_table[i]->status_flags->flags |
 					    O_NONBLOCK);
diff --git a/lib/libpthread/uthread/uthread_socket.c b/lib/libpthread/uthread/uthread_socket.c
index 3cecc3b57da..f43564060e3 100644
--- a/lib/libpthread/uthread/uthread_socket.c
+++ b/lib/libpthread/uthread/uthread_socket.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_socket.c,v 1.4 2006/09/22 19:04:33 kurt Exp $	*/
+/*	$OpenBSD: uthread_socket.c,v 1.5 2006/09/26 14:18:28 kurt Exp $	*/
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>
  * All rights reserved.
@@ -50,7 +50,7 @@ socket(int af, int type, int protocol)
 		/* Error creating socket. */
 
 	/* Initialise the entry in the file descriptor table: */
-	} else if (_thread_fd_table_init(fd, NULL) != 0) {
+	} else if (_thread_fd_table_init(fd, FD_INIT_NEW, NULL) != 0) {
 		_thread_sys_close(fd);
 		fd = -1;
 	}
diff --git a/lib/libpthread/uthread/uthread_socketpair.c b/lib/libpthread/uthread/uthread_socketpair.c
index bd824bb867d..60f7db40338 100644
--- a/lib/libpthread/uthread/uthread_socketpair.c
+++ b/lib/libpthread/uthread/uthread_socketpair.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uthread_socketpair.c,v 1.5 2006/09/22 19:04:33 kurt Exp $	*/
+/*	$OpenBSD: uthread_socketpair.c,v 1.6 2006/09/26 14:18:28 kurt Exp $	*/
 /*
  * Copyright (c) 1995-1998 John Birrell <jb@cimlogic.com.au>
  * All rights reserved.
@@ -46,8 +46,8 @@ socketpair(int af, int type, int protocol, int pair[2])
 {
 	int             ret;
 	if (!((ret = _thread_sys_socketpair(af, type, protocol, pair)) < 0))
-		if (_thread_fd_table_init(pair[0], NULL) != 0 ||
-		    _thread_fd_table_init(pair[1], NULL) != 0) {
+		if (_thread_fd_table_init(pair[0], FD_INIT_NEW, NULL) != 0 ||
+		    _thread_fd_table_init(pair[1], FD_INIT_NEW, NULL) != 0) {
 			_thread_sys_close(pair[0]);
 			_thread_sys_close(pair[1]);
 			ret = -1;