* Already set tty::lock() in init_driver(). This makes it always safe to

be used.
* Added tty::ref_count. Each cookie keeps a reference. Only when a
  cookie is freed the reference is surrendered. A tty is considered used
  as long as it is still referenced. This allows to access a tty through
  the cookie, even if it already has been closed.
* Fixed tty_deselect(). It was keeping registered select events when
  called after the cookie has been closed. The referenced select_sync
  structure would become invalid and later attempts to send select
  notifications for the tty could crash. Fixes #3126.


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

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

@@ -31,6 +31,8 @@ char *gDeviceNames[kNumTTYs * 2 + 3];
 	// reserve space for "pt/" and "tt/" entries, "ptmx", "tty", and the
 	// terminating NULL
 
+static mutex sTTYLocks[kNumTTYs];
+
 struct mutex gGlobalTTYLock;
 struct mutex gTTYCookieLock;
 struct recursive_lock gTTYRequestLock;
@@ -84,8 +86,9 @@ init_driver(void)
 		if (++digit > 15)
 			digit = 0, letter++;
 
-		reset_tty(&gMasterTTYs[i], i, true);
+		mutex_init(&sTTYLocks[i], "tty lock");
-		reset_tty(&gSlaveTTYs[i], i, false);
+		reset_tty(&gMasterTTYs[i], i, &sTTYLocks[i], true);
+		reset_tty(&gSlaveTTYs[i], i, &sTTYLocks[i], false);
 		reset_tty_settings(&gTTYSettings[i], i);
 
 		if (!gDeviceNames[i] || !gDeviceNames[i + kNumTTYs]) {
@@ -113,6 +116,9 @@ uninit_driver(void)
 	for (int32 i = 0; i < (int32)kNumTTYs * 2; i++)
 		free(gDeviceNames[i]);
 
+	for (int32 i = 0; i < (int32)kNumTTYs; i++)
+		mutex_destroy(&sTTYLocks[i]);
+
 	recursive_lock_destroy(&gTTYRequestLock);
 	mutex_destroy(&gTTYCookieLock);
 	mutex_destroy(&gGlobalTTYLock);

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

@@ -20,7 +20,6 @@
 
 
 struct master_cookie : tty_cookie {
-	struct mutex	lock;
 };
 
 
@@ -43,11 +42,8 @@ create_master_cookie(master_cookie *&cookie, struct tty *master,
 	if (cookie == NULL)
 		return B_NO_MEMORY;
 
-	mutex_init(&cookie->lock, "tty lock");
-
 	status_t error = init_tty_cookie(cookie, master, slave, openMode);
 	if (error != B_OK) {
-		mutex_destroy(&cookie->lock);
 		free(cookie);
 		return error;
 	}
@@ -77,12 +73,12 @@ master_open(const char *name, uint32 flags, void **_cookie)
 
 	if (findUnusedTTY) {
 		for (index = 0; index < (int32)kNumTTYs; index++) {
-			if (gMasterTTYs[index].open_count == 0)
+			if (gMasterTTYs[index].ref_count == 0)
 				break;
 		}
 		if (index >= (int32)kNumTTYs)
 			return ENOENT;
-	} else if (gMasterTTYs[index].open_count > 0) {
+	} else if (gMasterTTYs[index].ref_count > 0) {
 		// we're already open!
 		return B_BUSY;
 	}
@@ -101,11 +97,8 @@ master_open(const char *name, uint32 flags, void **_cookie)
 		return status;
 	}
 
-	gMasterTTYs[index].lock = &cookie->lock;
-
 	add_tty_cookie(cookie);
 
-	
 	*_cookie = cookie;
 
 	return B_OK;
@@ -138,8 +131,10 @@ master_free_cookie(void *_cookie)
 	// The TTY is already closed. We only have to free the cookie.
 	master_cookie *cookie = (master_cookie *)_cookie;
 
+	MutexLocker globalLocker(gGlobalTTYLock);
 	uninit_tty_cookie(cookie);
-	mutex_destroy(&cookie->lock);
+	globalLocker.Unlock();
+
 	free(cookie);
 
 	return B_OK;

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

@@ -146,7 +146,10 @@ slave_free_cookie(void *_cookie)
 
 	TRACE(("slave_free_cookie: cookie %p\n", _cookie));
 
+	MutexLocker globalLocker(gGlobalTTYLock);
 	uninit_tty_cookie(cookie);
+	globalLocker.Unlock();
+
 	free(cookie);
 
 	return B_OK;

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

@@ -44,8 +44,8 @@
 
 	gGlobalTTYLock: Guards open/close operations. When held, tty_open(),
 	tty_close(), tty_close_cookie() etc. won't be invoked by other threads,
-	cookies won't be added to/removed from TTYs, and tty::open_count,
+	cookies won't be added to/removed from TTYs, and tty::ref_count,
-	tty_cookie::closed won't change.
+	tty::open_count, tty_cookie::closed won't change.
 
 	gTTYCookieLock: Guards the access to the fields
 	tty_cookie::{thread_count,closed}, or more precisely makes access to them
@@ -59,10 +59,7 @@
 	window_size,pgrp_id}}. Moreover when held guarantees that tty::open_count
 	won't drop to zero (both gGlobalTTYLock and tty::lock must be held to
 	decrement it). A tty and the tty connected to it (master and slave) share
-	the same lock. tty::lock is only valid when tty::open_count is > 0. So
+	the same lock.
-	before accessing tty::lock, it must be made sure that it is still valid.
-	Given a tty_cookie, TTYReference can be used to do that, or otherwise
-	gGlobalTTYLock can be acquired and tty::open_count be checked.
 
 	gTTYRequestLock: Guards access to tty::{reader,writer}_queue (most
 	RequestQueue methods do the locking themselves (the lock is a
@@ -820,11 +817,12 @@ reset_tty_settings(tty_settings *settings, int32 index)
 
 
 void
-reset_tty(struct tty *tty, int32 index, bool isMaster)
+reset_tty(struct tty *tty, int32 index, mutex* lock, bool isMaster)
 {
+	tty->ref_count = 0;
 	tty->open_count = 0;
 	tty->index = index;
-	tty->lock = NULL;
+	tty->lock = lock;
 	tty->settings = &gTTYSettings[index];
 	tty->select_pool = NULL;
 	tty->is_master = isMaster;
@@ -946,9 +944,8 @@ tty_input_putc(struct tty *tty, int c)
 #endif // 0
 
 
-/**
+/*!	The global lock must be held.
- * The global lock must be held.
+*/
- */
 status_t
 init_tty_cookie(tty_cookie *cookie, struct tty *tty, struct tty *otherTTY,
 	uint32 openMode)
@@ -963,13 +960,19 @@ init_tty_cookie(tty_cookie *cookie, struct tty *tty, struct tty *otherTTY,
 	cookie->thread_count = 0;
 	cookie->closed = false;
 
+	tty->ref_count++;
+
 	return B_OK;
 }
 
 
+/*!	The global lock must be held.
+*/
 void
 uninit_tty_cookie(tty_cookie *cookie)
 {
+	cookie->tty->ref_count--;
+
 	if (cookie->blocking_semaphore >= 0) {
 		delete_sem(cookie->blocking_semaphore);
 		cookie->blocking_semaphore = -1;
@@ -1081,8 +1084,8 @@ tty_close_cookie(struct tty_cookie *cookie)
 			requestOwner.Wait(false);
 
 			// re-lock
-			ttyLocker.SetTo(cookie->tty->lock, false);
+			ttyLocker.Lock();
-			requestLocker.SetTo(gTTYRequestLock, false);
+			requestLocker.Lock();
 
 			// dequeue our request
 			requestOwner.Dequeue();
@@ -1094,6 +1097,8 @@ tty_close_cookie(struct tty_cookie *cookie)
 		tty_close(cookie->tty);
 	}
 
+	// notify pending select()s and cleanup the select sync pool
+
 	// notify a select write event on the other tty, if we've closed this tty
 	if (cookie->tty->open_count == 0 && cookie->other_tty->open_count > 0)
 		tty_notify_select_event(cookie->other_tty, B_SELECT_WRITE);
@@ -1306,7 +1311,6 @@ tty_open(struct tty *tty, tty_service_func func)
 	if (init_line_buffer(tty->input_buffer, TTY_BUFFER_SIZE) < B_OK)
 		return B_NO_MEMORY;
 
-	tty->lock = NULL;
 	tty->service_func = func;
 
 	// construct the queues
@@ -1943,14 +1947,6 @@ tty_deselect(tty_cookie *cookie, uint8 event, selectsync *sync)
 	if (event < B_SELECT_READ || event > B_SELECT_ERROR)
 		return B_BAD_VALUE;
 
-	// If the TTY is already closed, we're done. Note that we don't use a
-	// TTYReference here, but acquire the global lock, since we don't want
-	// return before tty_close_cookie() is done (it sends out the select
-	// events on close and our select() could miss one, if we don't wait).
-	MutexLocker globalLocker(gGlobalTTYLock);
-	if (cookie->closed)
-		return B_OK;
-
 	// lock the TTY (guards the select sync pool, among other things)
 	MutexLocker ttyLocker(tty->lock);
 

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

@@ -129,6 +129,7 @@ struct tty_settings {
 };
 
 struct tty {
+	int32				ref_count;	// referenced by cookies
 	int32				open_count;
 	int32				index;
 	struct mutex*		lock;
@@ -162,7 +163,7 @@ extern struct recursive_lock gTTYRequestLock;
 // functions available for master/slave TTYs
 
 extern int32 get_tty_index(const char *name);
-extern void reset_tty(struct tty *tty, int32 index, bool isMaster);
+extern void reset_tty(struct tty *tty, int32 index, mutex* lock, bool isMaster);
 extern void reset_tty_settings(tty_settings *settings, int32 index);
 //extern status_t tty_input_putc(struct tty *tty, int c);
 extern status_t tty_input_read(tty_cookie *cookie, void *buffer,