From 5cf12c68127dec025f91fde8e98d13279e24014d Mon Sep 17 00:00:00 2001
From: Alberto Bertogli <albertogli@telpin.com.ar>
Date: Thu, 24 May 2007 04:41:12 +0000
Subject: [PATCH] Handle fd reuse almost properly. This patch handles the
 following scenario:

	- fd0 locks l1 -> success
	- fd1 locks l1 -> blocks
	- fd1 closes
	- new connection, gets fd1
	- fd0 unlocks l1 -> success
		-> fd1 gets notified of the l1 assigment, but if never asked
		for it.

That's because when an fd gets closed, it doesn't remove itself from the
queues it's sitting on.

The patch is weak, but has been extensively tested, so it's going in as a
temporary fix.

Thanks to Lloyd C. Cha for the report, testing, and help with the debugging.
---
 include/lock.h   |  1 +
 include/wqueue.h |  2 +-
 lock.c           | 13 +++++++++++++
 net.c            | 27 ++++++++++++++++++++++++++-
 wqueue.c         |  8 ++++----
 5 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/include/lock.h b/include/lock.h
index 1ea3088..ff0c9f0 100644
--- a/include/lock.h
+++ b/include/lock.h
@@ -5,5 +5,6 @@ int lock_acquire(char *s, int fd);
 int lock_trylock(char *s, int fd);
 int lock_release(char *s);
 int lock_set_fd(char *s, int fd);
+int lock_remove_fd(char *s, int fd);
 
 #endif
diff --git a/include/wqueue.h b/include/wqueue.h
index 2394329..024bcdc 100644
--- a/include/wqueue.h
+++ b/include/wqueue.h
@@ -10,7 +10,7 @@ struct wqentry {
 };
 
 struct wqentry *wq_add(struct wqentry *wq, int fd);
-int wq_del(struct wqentry *wq, int fd);
+struct wqentry *wq_del(struct wqentry *wq, int fd);
 
 
 #endif
diff --git a/lock.c b/lock.c
index ce25a25..fe9323a 100644
--- a/lock.c
+++ b/lock.c
@@ -94,3 +94,16 @@ int lock_set_fd(char *s, int fd) {
 	return rv;
 }
 
+/* remove a given fd from the lock's waitqueue */
+int lock_remove_fd(char *s, int fd) {
+	struct hentry *h;
+
+	h = hash_lookup(s, 0);
+	if (h == NULL)
+		return -1;
+
+	h->wq = wq_del(h->wq, fd);
+	return 1;
+}
+
+
diff --git a/net.c b/net.c
index a5779e0..7e05cf1 100644
--- a/net.c
+++ b/net.c
@@ -50,6 +50,9 @@ static struct part_buf bufs[MAXCONN];
 /* owned locks per fd */
 static struct list *olocks[MAXCONN];
 
+/* queued locks per fd */
+static struct list *qlocks[MAXCONN];
+
 
 /*
  * thread structures
@@ -436,14 +439,31 @@ static int net_close(int fd) {
 	/* move open locks to the orphan list */
 	if (olocks[fd] != NULL) {
 		/* first, mark its open locks as orphans */
-		for (p = olocks[fd]; p != NULL; p = p->next)
+		for (p = olocks[fd]; p != NULL; p = p->next) {
 			lock_set_fd(p->name, -1);
+		}
 
 		/* and then move them to the orphan list */
 		orphans = list_mult_add(orphans, olocks[fd]);
 		olocks[fd] = NULL;
 	}
 
+	/* remove ourselves from the waiting queues */
+	if (qlocks[fd] != NULL) {
+		p = qlocks[fd];
+		while (p != NULL) {
+			/* FIXME: we don't take locks over p->name, so this is
+			 * not really threadsafe. Taking locks is tricky
+			 * because we could be called with some p->name
+			 * locked. */
+			lock_remove_fd(p->name, fd);
+
+			/* fragile, because list_remove() frees p->name */
+			p = list_remove(p, p->name);
+		}
+		qlocks[fd] = NULL;
+	}
+
 	return close(fd);
 }
 
@@ -471,6 +491,7 @@ static int net_parse(int fd, struct net_cmd *cmd) {
 				olocks[fd] = list_add(olocks[fd], data);
 				send_op = REP_LOCK_ACQUIRED;
 			} else {
+				qlocks[fd] = list_add(qlocks[fd], data);
 				send_op = REP_ACK;
 			}
 			hash_unlock_chain(data);
@@ -576,6 +597,10 @@ int net_wakeup(struct hentry *h) {
 		/* add the open lock; the caller will take care to remove from the
 		 * other queue */
 		olocks[fd] = list_add(olocks[fd], h->objname);
+
+		/* and remove the entry in the fd's queued locks */
+		qlocks[fd] = list_remove(qlocks[fd], h->objname);
+
 		return 1;
 
 	} while (p != NULL);
diff --git a/wqueue.c b/wqueue.c
index b0e7c28..9274a9b 100644
--- a/wqueue.c
+++ b/wqueue.c
@@ -29,7 +29,7 @@ struct wqentry *wq_add(struct wqentry *wq, int fd) {
 }
 
 /* removes a given fd from the waitqueue */
-int wq_del(struct wqentry *wq, int fd) {
+struct wqentry *wq_del(struct wqentry *wq, int fd) {
 	struct wqentry *p, *prev;
 	prev = NULL;
 
@@ -43,13 +43,13 @@ int wq_del(struct wqentry *wq, int fd) {
 				prev->next = p->next;
 			}
 			free(p);
-			return 1;
+			return wq;
 		}
 		prev = p;
 	}
 
-	/* we couldn't find a match!!! */
-	return 0;
+	/* we couldn't find a match, return the same queue */
+	return wq;
 }