* Dealt with (hopefully) all open/close race conditions.

* Implemented unblocking of pending operations when closing a cookie.
  Needed to change the simple one reader semaphore, one writer
  semaphore strategy for blocking to a request queue based approach
  (otherwise unblocking would have been really hairy).
* Implemented select() support.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@11931 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/kernel/drivers/tty/driver.cpp

@@ -3,12 +3,16 @@
 ** Distributed under the terms of the Haiku License.
 */
 
+#include <new>
 
-#include "tty_private.h"
-
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <stdio.h>
+
+#include <lock.h>
+
+#include "SemaphorePool.h"
+#include "tty_private.h"
 
 
 //#define TTY_TRACE
@@ -20,12 +24,19 @@
 
 #define DRIVER_NAME "tty"
 
+static const int kMaxCachedSemaphores = 8;
 
 int32 api_version = B_CUR_DRIVER_API_VERSION;
 
 static char *sDeviceNames[kNumTTYs * 2 + 1];
 	// reserve space for "pt/" and "tt/" entries and the terminating NULL
 
+struct mutex gGlobalTTYLock;
+struct mutex gTTYCookieLock;
+struct recursive_lock gTTYRequestLock;
+static char sSemaphorePoolBuffer[sizeof(SemaphorePool)];
+SemaphorePool *gSemaphorePool = 0;
+
 
 status_t
 init_hardware(void)
@@ -40,6 +51,37 @@ init_driver(void)
 {
 	TRACE((DRIVER_NAME ": init_driver()\n"));
 
+	memset(sDeviceNames, 0, sizeof(sDeviceNames));
+
+	// create the global mutex
+	status_t error = mutex_init(&gGlobalTTYLock, "tty global");
+	if (error != B_OK)
+		return error;
+
+	// create the cookie mutex
+	error = mutex_init(&gTTYCookieLock, "tty cookies");
+	if (error != B_OK) {
+		mutex_destroy(&gGlobalTTYLock);
+		return error;
+	}
+
+	// create the request mutex
+	error = recursive_lock_init(&gTTYRequestLock, "tty requests");
+	if (error != B_OK) {
+		mutex_destroy(&gTTYCookieLock);
+		mutex_destroy(&gGlobalTTYLock);
+		return error;
+	}
+
+	// create the semaphore pool
+	gSemaphorePool
+		= new(sSemaphorePoolBuffer) SemaphorePool(kMaxCachedSemaphores);
+	error = gSemaphorePool->Init();
+	if (error != B_OK) {
+		uninit_driver();
+		return error;
+	}
+
 	// create driver name array and initialize basic TTY structures
 
 	char letter = 'p';
@@ -62,9 +104,12 @@ init_driver(void)
 
 		reset_tty(&gMasterTTYs[i], i);
 		reset_tty(&gSlaveTTYs[i], i);
-	}
 
-	sDeviceNames[kNumTTYs * 2] = NULL;
+		if (!sDeviceNames[i] || !sDeviceNames[i + kNumTTYs]) {
+			uninit_driver();
+			return B_NO_MEMORY;
+		}
+	}
 
 	return B_OK;
 }
@@ -75,8 +120,17 @@ uninit_driver(void)
 {
 	TRACE((DRIVER_NAME ": uninit_driver()\n"));
 
-	for (int32 i = 0; sDeviceNames[i] != NULL; i++)
+	for (int32 i = 0; i < (int32)kNumTTYs * 2; i++)
 		free(sDeviceNames[i]);
+
+	if (gSemaphorePool) {
+		gSemaphorePool->~SemaphorePool();
+		gSemaphorePool = NULL;
+	}
+
+	recursive_lock_destroy(&gTTYRequestLock);
+	mutex_destroy(&gTTYCookieLock);
+	mutex_destroy(&gGlobalTTYLock);
 }
 
 

src/add-ons/kernel/drivers/tty/master.cpp

@@ -7,6 +7,7 @@
 #include "tty_private.h"
 
 #include <stdlib.h>
+#include <util/AutoLock.h>
 
 
 //#define MASTER_TRACE
@@ -17,10 +18,8 @@
 #endif
 
 
-struct master_cookie {
-	struct tty	*tty;
-	struct tty	*slave;
-	uint32		open_mode;
+struct master_cookie : tty_cookie {
+	struct mutex	lock;
 };
 
 
@@ -35,6 +34,31 @@ master_service(struct tty *tty, uint32 op)
 }
 
 
+static status_t
+create_master_cookie(master_cookie *&cookie, struct tty *master,
+	struct tty *slave, uint32 openMode)
+{
+	cookie = (master_cookie*)malloc(sizeof(struct master_cookie));
+	if (cookie == NULL)
+		return B_NO_MEMORY;
+
+	status_t error = mutex_init(&cookie->lock, "tty lock");
+	if (error != B_OK) {
+		free(cookie);
+		return error;
+	}
+
+	error = init_tty_cookie(cookie, master, slave, openMode);
+	if (error != B_OK) {
+		mutex_destroy(&cookie->lock);
+		free(cookie);
+		return error;
+	}
+
+	return B_OK;
+}
+
+
 //	#pragma mark -
 
 
@@ -47,28 +71,32 @@ master_open(const char *name, uint32 flags, void **_cookie)
 
 	TRACE(("master_open: TTY index = %ld (name = %s)\n", index, name));
 
-	if (atomic_or(&gMasterTTYs[index].open_count, 1) != 0) {
+	MutexLocker globalLocker(gGlobalTTYLock);
+
+	if (gMasterTTYs[index].open_count > 0) {
 		// we're already open!
 		return B_BUSY;
 	}
 
 	status_t status = tty_open(&gMasterTTYs[index], &master_service);
 	if (status < B_OK) {
-		// initializing TTY failed, reset open counter
-		atomic_and(&gMasterTTYs[index].open_count, 0);
+		// initializing TTY failed
 		return status;
 	}
 
-	master_cookie *cookie = (master_cookie *)malloc(sizeof(struct master_cookie));
-	if (cookie == NULL) {
-		atomic_and(&gMasterTTYs[index].open_count, 0);
+	master_cookie *cookie;
+	status = create_master_cookie(cookie, &gMasterTTYs[index],
+		&gSlaveTTYs[index], flags);
+	if (status != B_OK) {
 		tty_close(&gMasterTTYs[index]);
-		return B_NO_MEMORY;
+		return status;
 	}
 
-	cookie->tty = &gMasterTTYs[index];
-	cookie->slave = &gSlaveTTYs[index];
-	cookie->open_mode = flags;
+	gMasterTTYs[index].lock = &cookie->lock;
+
+	add_tty_cookie(cookie);
+
+	
 	*_cookie = cookie;
 
 	return B_OK;
@@ -82,7 +110,15 @@ master_close(void *_cookie)
 
 	TRACE(("master_close: cookie %p\n", _cookie));
 
-	atomic_and(&cookie->tty->open_count, 0);
+	MutexLocker globalLocker(gGlobalTTYLock);
+
+	// close all connected slave cookies first
+	while (tty_cookie *slave = cookie->other_tty->cookies.Head())
+		tty_close_cookie(slave);
+
+	// close the client cookie
+	tty_close_cookie(cookie);
+
 	return B_OK;
 }
 
@@ -90,10 +126,13 @@ master_close(void *_cookie)
 static status_t
 master_free_cookie(void *_cookie)
 {
+	// The TTY is already closed. We only have to free the cookie.
 	master_cookie *cookie = (master_cookie *)_cookie;
 
-	tty_close(cookie->tty);
+	uninit_tty_cookie(cookie);
+	mutex_destroy(&cookie->lock);
 	free(cookie);
+
 	return B_OK;
 }
 
@@ -105,19 +144,7 @@ master_ioctl(void *_cookie, uint32 op, void *buffer, size_t length)
 
 	TRACE(("master_ioctl: cookie %p, op %lu, buffer %p, length %lu\n", _cookie, op, buffer, length));
 
-	switch (op) {
-		case B_SET_BLOCKING_IO:
-			cookie->open_mode &= ~O_NONBLOCK;
-			break;
-		case B_SET_NONBLOCKING_IO:
-			cookie->open_mode |= O_NONBLOCK;
-			break;
-
-		default:
-			return tty_ioctl(cookie->tty, op, buffer, length);		
-	}
-
-	return B_OK;
+	return tty_ioctl(cookie, op, buffer, length);		
 }
 
 
@@ -126,8 +153,15 @@ master_read(void *_cookie, off_t offset, void *buffer, size_t *_length)
 {
 	master_cookie *cookie = (master_cookie *)_cookie;
 
-	TRACE(("master_read: cookie %p, offset %Ld, buffer %p, length %lu\n", _cookie, offset, buffer, *_length));
-	return tty_input_read(cookie->tty, buffer, _length, cookie->open_mode);
+	TRACE(("master_read: cookie %p, offset %Ld, buffer %p, length %lu\n",
+		_cookie, offset, buffer, *_length));
+
+	status_t result = tty_input_read(cookie, buffer, _length);
+
+	TRACE(("master_read done: cookie %p, result: %lx, length %lu\n", _cookie,
+		result, *_length));
+
+	return result;
 }
 
 
@@ -136,8 +170,15 @@ master_write(void *_cookie, off_t offset, const void *buffer, size_t *_length)
 {
 	master_cookie *cookie = (master_cookie *)_cookie;
 
-	TRACE(("master_write: cookie %p, offset %Ld, buffer %p, length %lu\n", _cookie, offset, buffer, *_length));
-	return tty_write_to_tty(cookie->tty, cookie->slave, buffer, _length, cookie->open_mode, true);
+	TRACE(("master_write: cookie %p, offset %Ld, buffer %p, length %lu\n",
+		_cookie, offset, buffer, *_length));
+
+	status_t result = tty_write_to_tty(cookie, buffer, _length, true);
+
+	TRACE(("master_write done: cookie %p, result: %lx, length %lu\n", _cookie,
+		result, *_length));
+
+	return result;
 }
 
 
@@ -146,7 +187,7 @@ master_select(void *_cookie, uint8 event, uint32 ref, selectsync *sync)
 {
 	master_cookie *cookie = (master_cookie *)_cookie;
 
-	return tty_select(cookie->tty, event, ref, sync);
+	return tty_select(cookie, event, ref, sync);
 }
 
 
@@ -155,7 +196,7 @@ master_deselect(void *_cookie, uint8 event, selectsync *sync)
 {
 	master_cookie *cookie = (master_cookie *)_cookie;
 
-	return tty_deselect(cookie->tty, event, sync);
+	return tty_deselect(cookie, event, sync);
 }
 
 

src/add-ons/kernel/drivers/tty/slave.cpp

@@ -4,10 +4,12 @@
 */
 
 
-#include "tty_private.h"
-
 #include <stdlib.h>
 
+#include <util/AutoLock.h>
+
+#include "tty_private.h"
+
 
 //#define SLAVE_TRACE
 #ifdef SLAVE_TRACE
@@ -17,10 +19,7 @@
 #endif
 
 
-struct slave_cookie {
-	struct tty	*tty;
-	struct tty	*master;
-	uint32		open_mode;
+struct slave_cookie : tty_cookie {
 };
 
 
@@ -36,32 +35,44 @@ slave_open(const char *name, uint32 flags, void **_cookie)
 
 	TRACE(("slave_open: TTY index = %ld (name = %s)\n", index, name));
 
+	MutexLocker globalLocker(gGlobalTTYLock);
+
 	// we may only be used if our master has already been opened
 	if (gMasterTTYs[index].open_count == 0)
 		return B_IO_ERROR;
 
-	if (atomic_add(&gSlaveTTYs[index].open_count, 1) == 0) {
-		// ToDo: this is broken and must be synchronized with all
-		//	other open calls to that slave
-		// Also, tty_open() should probably be changed and always called in open().
+	if (gSlaveTTYs[index].open_count == 0) {
 		status_t status = tty_open(&gSlaveTTYs[index], NULL);
 		if (status < B_OK) {
-			// initializing TTY failed, reset open counter
-			atomic_add(&gSlaveTTYs[index].open_count, -1);
+			// initializing TTY failed
 			return status;
 		}
 	}
 
  	slave_cookie *cookie = (slave_cookie *)malloc(sizeof(struct slave_cookie));
 	if (cookie == NULL) {
-		atomic_add(&gSlaveTTYs[index].open_count, -1);
-		tty_close(&gSlaveTTYs[index]);
+		if (gSlaveTTYs[index].open_count == 0)
+			tty_close(&gSlaveTTYs[index]);
+
 		return B_NO_MEMORY;
 	}
 
-	cookie->tty = &gSlaveTTYs[index];
-	cookie->master = &gMasterTTYs[index];
-	cookie->open_mode = flags;
+	status_t status = init_tty_cookie(cookie, &gSlaveTTYs[index],
+		&gMasterTTYs[index], flags);
+	if (status != B_OK) {
+		free(cookie);
+
+		if (gSlaveTTYs[index].open_count == 0)
+			tty_close(&gSlaveTTYs[index]);
+
+		return status;
+	}
+
+	if (gSlaveTTYs[index].open_count == 0)
+		gSlaveTTYs[index].lock = gMasterTTYs[index].lock;
+
+	add_tty_cookie(cookie);
+
 	*_cookie = cookie;
 
 	return B_OK;
@@ -71,6 +82,15 @@ slave_open(const char *name, uint32 flags, void **_cookie)
 static status_t
 slave_close(void *_cookie)
 {
+	slave_cookie *cookie = (slave_cookie *)_cookie;
+
+	TRACE(("slave_close: cookie %p\n", _cookie));
+
+	MutexLocker globalLocker(gGlobalTTYLock);
+
+	// unblock and wait for all blocking operations
+	tty_close_cookie(cookie);
+
 	return B_OK;
 }
 
@@ -78,14 +98,14 @@ slave_close(void *_cookie)
 static status_t
 slave_free_cookie(void *_cookie)
 {
+	// The TTY is already closed. We only have to free the cookie.
 	slave_cookie *cookie = (slave_cookie *)_cookie;
 
-	TRACE(("slave_close: cookie %p\n", _cookie));
-
-	if (atomic_add(&cookie->tty->open_count, -1) == 1)
-		tty_close(cookie->tty);
+	TRACE(("slave_free_cookie: cookie %p\n", _cookie));
 
+	uninit_tty_cookie(cookie);
 	free(cookie);
+
 	return B_OK;
 }
 
@@ -97,19 +117,7 @@ slave_ioctl(void *_cookie, uint32 op, void *buffer, size_t length)
 
 	TRACE(("slave_ioctl: cookie %p, op %lu, buffer %p, length %lu\n", _cookie, op, buffer, length));
 
-	switch (op) {
-		case B_SET_BLOCKING_IO:
-			cookie->open_mode &= ~O_NONBLOCK;
-			break;
-		case B_SET_NONBLOCKING_IO:
-			cookie->open_mode |= O_NONBLOCK;
-			break;
-
-		default:
-			return tty_ioctl(cookie->tty, op, buffer, length);		
-	}
-
-	return B_OK;
+	return tty_ioctl(cookie, op, buffer, length);		
 }
 
 
@@ -118,8 +126,15 @@ slave_read(void *_cookie, off_t offset, void *buffer, size_t *_length)
 {
 	slave_cookie *cookie = (slave_cookie *)_cookie;
 
-	TRACE(("slave_read: cookie %p, offset %Ld, buffer %p, length %lu\n", _cookie, offset, buffer, *_length));
-	return tty_input_read(cookie->tty, buffer, _length, cookie->open_mode);
+	TRACE(("slave_read: cookie %p, offset %Ld, buffer %p, length %lu\n",
+		_cookie, offset, buffer, *_length));
+
+	status_t result = tty_input_read(cookie, buffer, _length);
+
+	TRACE(("slave_read done: cookie %p, result %lx, length %lu\n",
+		_cookie, result, *_length));
+
+	return result;
 }
 
 
@@ -128,22 +143,33 @@ slave_write(void *_cookie, off_t offset, const void *buffer, size_t *_length)
 {
 	slave_cookie *cookie = (slave_cookie *)_cookie;
 
-	TRACE(("slave_write: cookie %p, offset %Ld, buffer %p, length %lu\n", _cookie, offset, buffer, *_length));
-	return tty_write_to_tty(cookie->tty, cookie->master, buffer, _length, cookie->open_mode, false);
+	TRACE(("slave_write: cookie %p, offset %Ld, buffer %p, length %lu\n",
+		_cookie, offset, buffer, *_length));
+
+	status_t result = tty_write_to_tty(cookie, buffer, _length, false);
+
+	TRACE(("slave_write done: cookie %p, result %lx, length %lu\n",
+		_cookie, result, *_length));
+
+	return result;
 }
 
 
 static status_t
 slave_select(void *_cookie, uint8 event, uint32 ref, selectsync *sync)
 {
-	return B_OK;
+	slave_cookie *cookie = (slave_cookie *)_cookie;
+
+	return tty_select(cookie, event, ref, sync);
 }
 
 
 static status_t
 slave_deselect(void *_cookie, uint8 event, selectsync *sync)
 {
-	return B_OK;
+	slave_cookie *cookie = (slave_cookie *)_cookie;
+
+	return tty_deselect(cookie, event, sync);
 }
 
 

src/add-ons/kernel/drivers/tty/tty_private.h

@@ -9,6 +9,8 @@
 #include <KernelExport.h>
 #include <Drivers.h>
 #include <lock.h>
+#include <fs/select_sync_pool.h>
+#include <util/DoublyLinkedList.h>
 
 #include <termios.h>
 
@@ -24,12 +26,105 @@
 typedef status_t (*tty_service_func)(struct tty *tty, uint32 op);
 	// not yet used...
 
+
+class RequestOwner;
+class Semaphore;
+class SemaphorePool;
+struct tty;
+struct tty_cookie;
+
+class Request : public DoublyLinkedListLinkImpl<Request> {
+public:
+	Request();
+
+	void Init(RequestOwner *owner, tty_cookie *cookie, int32 bytesNeeded);
+
+	tty_cookie *TTYCookie() const	{ return fCookie; }
+
+	void Notify(int32 bytesAvailable);
+	void NotifyError(status_t error);
+
+	bool WasNotified() const		{ return fNotified; }
+	bool HasError() const			{ return fError; }
+
+private:
+	RequestOwner	*fOwner;
+	tty_cookie		*fCookie;
+	int32			fBytesNeeded;
+	bool			fNotified;
+	bool			fError;
+};
+
+class RequestQueue {
+public:
+	RequestQueue();
+	~RequestQueue()	{}
+
+	void Add(Request *request);
+	void Remove(Request *request);
+
+	Request *First() const				{ return fRequests.First(); }
+	bool IsEmpty() const				{ return fRequests.IsEmpty(); }
+
+	void NotifyFirst(int32 bytesAvailable);
+	void NotifyError(status_t error);
+	void NotifyError(tty_cookie *cookie, status_t error);
+
+private:
+	typedef DoublyLinkedList<Request> RequestList;
+
+	RequestList	fRequests;
+};
+
+class RequestOwner {
+public:
+	RequestOwner();
+
+	void Enqueue(tty_cookie *cookie, RequestQueue *queue1, 
+		RequestQueue *queue2 = NULL);
+	void Dequeue();
+
+	void SetBytesNeeded(int32 bytesNeeded);
+	int32 BytesNeeded() const	{ return fBytesNeeded; }
+
+	status_t Wait(bool interruptable, Semaphore *sem = NULL);
+
+	bool IsFirstInQueues();
+
+	void Notify(Request *request);
+	void NotifyError(Request *request, status_t error);
+
+	status_t Error() const	{ return fError; }
+
+private:
+	Semaphore		*fSemaphore;
+	tty_cookie		*fCookie;
+	status_t		fError;
+	RequestQueue	*fRequestQueues[2];
+	Request			fRequests[2];
+	int32			fBytesNeeded;
+};
+
+
+struct tty_cookie : DoublyLinkedListLinkImpl<tty_cookie> {
+	struct tty			*tty;
+	struct tty			*other_tty;
+	uint32				open_mode;
+	select_sync_pool	*select_pool;
+	int32				thread_count;
+	sem_id				blocking_semaphore;
+	bool				closed;
+};
+
+typedef DoublyLinkedList<tty_cookie> TTYCookieList;
+
 struct tty {
 	int32				open_count;
 	int32				index;
-	struct mutex		lock;
-	sem_id				read_sem;
-	sem_id				write_sem;
+	struct mutex		*lock;
+	RequestQueue		reader_queue;
+	RequestQueue		writer_queue;
+	TTYCookieList		cookies;
 	pid_t				pgrp_id;
 	line_buffer			input_buffer;
 	tty_service_func	service_func;
@@ -45,21 +140,35 @@ extern tty gSlaveTTYs[kNumTTYs];
 extern device_hooks gMasterTTYHooks;
 extern device_hooks gSlaveTTYHooks;
 
+extern struct mutex gGlobalTTYLock;
+extern struct mutex gTTYCookieLock;
+extern struct recursive_lock gTTYRequestLock;
+extern SemaphorePool *gSemaphorePool;
+
 
 // functions available for master/slave TTYs
 
 extern int32 get_tty_index(const char *name);
 extern void reset_tty(struct tty *tty, int32 index);
-extern status_t tty_input_putc(struct tty *tty, int c);
-extern status_t tty_input_read(struct tty *tty, void *buffer, size_t *_length, uint32 mode);
+//extern status_t tty_input_putc(struct tty *tty, int c);
+extern status_t tty_input_read(tty_cookie *cookie, void *buffer,
+					size_t *_length);
 extern status_t tty_output_getc(struct tty *tty, int *_c);
-extern status_t tty_write_to_tty(struct tty *tty, struct tty *dest, const void *buffer,
-					size_t *_length, uint32 mode, bool sourceIsMaster);
+extern status_t tty_write_to_tty(tty_cookie *sourceCookie, const void *buffer,
+					size_t *_length, bool sourceIsMaster);
+
+extern status_t init_tty_cookie(tty_cookie *cookie, struct tty *tty,
+					struct tty *otherTTY, uint32 openMode);
+extern void uninit_tty_cookie(tty_cookie *cookie);
+extern void add_tty_cookie(tty_cookie *cookie);
+extern void tty_close_cookie(struct tty_cookie *cookie);
 
 extern status_t tty_open(struct tty *tty, tty_service_func func);
 extern status_t tty_close(struct tty *tty);
-extern status_t tty_ioctl(struct tty *tty, uint32 op, void *buffer, size_t length);
-extern status_t tty_select(struct tty *tty, uint8 event, uint32 ref, selectsync *sync);
-extern status_t tty_deselect(struct tty *tty, uint8 event, selectsync *sync);
+extern status_t tty_ioctl(tty_cookie *cookie, uint32 op, void *buffer,
+					size_t length);
+extern status_t tty_select(tty_cookie *cookie, uint8 event, uint32 ref,
+					selectsync *sync);
+extern status_t tty_deselect(tty_cookie *cookie, uint8 event, selectsync *sync);
 
 #endif	/* TTY_PRIVATE_H */