ovdb bug squashed

[Heath Kehoe]

Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
	charset=US-ASCII;
	format=flowed
I found a pretty significant bug in ovdb, that causes a deadlock that 
usually happens during an expireover run, but is actually caused when 
an nnrpd exits without closing an opened OVsearch handle (which can 
happen if the client disconnects during an XOVER).

Anyone running ovdb needs this fix.  It will hopefully appear in the 
CURRENT and STABLE trees soon.  Also, attached a patch that can be 
applied to 2.4.1 (maybe 2.4.0 also, I didn't test that).

-heath



-- Attached file included as plaintext by Ecartis --
-- File: diff.txt

diff -ru ../inn-2.4.1-dist/frontends/ovdb_server.c ./frontends/ovdb_server.c
--- ../inn-2.4.1-dist/frontends/ovdb_server.c	2004-01-07 16:47:19.000000000 -0600
+++ ./frontends/ovdb_server.c	2004-02-17 14:53:13.000000000 -0600
@@ -67,6 +67,7 @@
     int bufpos;
     void *buf;
     time_t lastactive;
+    void *currentsearch;
 };
 
 static struct reader *readertab;
@@ -172,11 +173,17 @@
 
     /*syslog(LOG_DEBUG, "OVDB: rs: do_opensrch '%s' %d %d", group, cmd->artlo, cmd->arthi);*/
 
-    reply->handle = ovdb_opensearch(group, cmd->artlo, cmd->arthi);
-    if(reply->handle == NULL) {
-    	reply->status = CMD_OPENSRCH | RPLY_ERROR;
+    if(r->currentsearch != NULL) {
+	/* can only open one search at a time */
+	reply->status = CMD_OPENSRCH | RPLY_ERROR;
     } else {
-    	reply->status = CMD_OPENSRCH;
+	reply->handle = ovdb_opensearch(group, cmd->artlo, cmd->arthi);
+	if(reply->handle == NULL) {
+	    reply->status = CMD_OPENSRCH | RPLY_ERROR;
+	} else {
+	    reply->status = CMD_OPENSRCH;
+	}
+	r->currentsearch = reply->handle;
     }
     free(r->buf);
     r->buf = reply;
@@ -226,6 +233,7 @@
     r->buf = NULL;
     r->bufpos = r->buflen = 0;
     r->mode = MODE_READ;
+    r->currentsearch = NULL;
 }
 
 static void
@@ -370,12 +378,17 @@
 newclient(int fd)
 {
     struct reader *r;
+    int i;
 
     nonblocking(fd, 1);
 
     if(numreaders >= readertablen) {
     	readertablen += 50;
         readertab = xrealloc(readertab, readertablen * sizeof(struct reader));
+        for(i = numreaders; i < readertablen; i++) {
+	    readertab[i].mode = MODE_CLOSED;
+	    readertab[i].buf = NULL;
+	}
     }
 
     r = &(readertab[numreaders]);
@@ -387,6 +400,7 @@
     r->bufpos = 0;
     r->buf = xstrdup(OVDB_SERVER_BANNER);
     r->lastactive = now;
+    r->currentsearch = NULL;
 
     handle_write(r);
 }
@@ -403,6 +417,10 @@
     if(r->buf != NULL) {
     	free(r->buf);
     }
+    if(r->currentsearch != NULL) {
+	ovdb_closesearch(r->currentsearch);
+	r->currentsearch = NULL;
+    }
 
     /* numreaders will get decremented by the calling function */
     for(i = which; i < numreaders-1; i++)
diff -ru ../inn-2.4.1-dist/storage/ovdb/ovdb.c ./storage/ovdb/ovdb.c
--- ../inn-2.4.1-dist/storage/ovdb/ovdb.c	2004-01-07 16:47:19.000000000 -0600
+++ ./storage/ovdb/ovdb.c	2004-02-17 15:12:48.000000000 -0600
@@ -1,8 +1,12 @@
 /*
  * ovdb.c
- * ovdb 2.00 beta4
- * Overview storage using BerkeleyDB 2.x/3.x
+ * ovdb 2.00
+ * Overview storage using BerkeleyDB 2.x/3.x/4.x
  *
+ * 2004-02-17 : Need to track search cursors, since it's possible that
+ *              ovdb_closesearch does not get called.  We now close
+ *              any cursors still open in ovdb_close, or they'd be in
+ *              the database indefinitely causing deadlocks.
  * 2002-08-13 : Change BOOL to bool, remove portability to < 2.4.
  * 2002-08-11 : Cleaned up use of sprintf and fixed a bunch of warnings.
  * 2002-02-28 : Update getartinfo for the overview API change in 2.4.  This
@@ -212,6 +216,7 @@
 	    if(r < 0 && errno == EINTR)
 		continue;
 	    syslog(LOG_ERR, "OVDB: rc: cant write: %m");
+	    clientfd = -1;
 	    exit(1);
 	}
 	p+= r;
@@ -232,6 +237,7 @@
 	    if(r < 0 && errno == EINTR)
 		continue;
 	    syslog(LOG_ERR, "OVDB: rc: cant read: %m");
+	    clientfd = -1;
 	    exit(1);
 	}
 	p+= r;
@@ -337,6 +343,7 @@
 
 	csend(&rs, sizeof rs);
     }
+    clientfd = -1;
 }
 
 
@@ -1998,6 +2005,13 @@
     int state;
 };
 
+/* Even though nnrpd only does one search at a time, a read server process could
+   do many concurrent searches; hence we must keep track of an arbitrary number of
+   open searches */
+static struct ovdbsearch **searches = NULL;
+static int nsearches = 0;
+static int maxsearches = 0;
+
 void *ovdb_opensearch(char *group, int low, int high)
 {
     DB *db;
@@ -2055,6 +2069,18 @@
     s->lastart = high;
     s->state = 0;
 
+    if(searches == NULL) {
+	nsearches = 0;
+	maxsearches = 50;
+	searches = xmalloc(sizeof(struct ovdbsearch *) * maxsearches);
+    }
+    if(nsearches == maxsearches) {
+	maxsearches += 50;
+	searches = xrealloc(searches, sizeof(struct ovdbsearch *) * maxsearches);
+    }
+    searches[nsearches] = s;
+    nsearches++;
+
     return (void *)s;
 }
 
@@ -2203,6 +2229,7 @@
 
 void ovdb_closesearch(void *handle)
 {
+    int i;
     if(clientmode) {
 	struct rs_cmd rs;
 
@@ -2216,6 +2243,16 @@
 	if(s->cursor)
 	    s->cursor->c_close(s->cursor);
 
+	for(i = 0; i < nsearches; i++) {
+	    if(s == searches[i]) {
+		break;
+	    }
+	}
+	nsearches--;
+	for( ; i < nsearches; i++) {
+	    searches[i] = searches[i+1];
+	}
+
 	free(handle);
     }
 }
@@ -2813,6 +2850,14 @@
 	return;
     }
 
+    while(searches != NULL && nsearches) {
+	ovdb_closesearch(searches[0]);
+    }
+    if(searches != NULL) {
+	free(searches);
+	searches = NULL;
+    }
+
     if(dbs) {
 	/* close databases */
 	for(i = 0; i < ovdb_conf.numdbfiles; i++)



-- Attached file included as plaintext by Ecartis --
-- File: PGP.sig
-- Desc: This is a digitally signed message part

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (Darwin)

iD8DBQFAMox7qvQcDdUKooERAsXTAJ9lGibWFb+EVg05gkppn3KBvBDXyACeJTXV
ncYZaxq9TvOSS6Vo1OgA4DY=
=uo6s
-----END PGP SIGNATURE-----