[news.software.nntp] NNTP 1.5.11 patch: history file checking

stevo@elroy.jpl.nasa.gov (Steve Groom) (02/14/91)

    Note:  The patch discussed and contained herein is NOT an
    "official" patch.  It is something that we have come up with to
    counter a problem observed in nntpd.  It has already been submitted
    to the keepers of NNTP.

We ran into some problems with nntpd when neighbors running nntplink
upgraded to v2.1.  Nntplink is designed to open a connection to a
remote nntpd and hold it open for a while.  It also contains some code
to force a disconnection from the remote nntpd after a certain period
of time.  In v2.1, that code is broken, and nntplink may keep the
connection open for a very long time (many hours, perhaps days).  A fix
for nntplink is in the works.  In the meantime, I've looked into the
problem from nntpd's perspective, a sort of defensive move.  You see,
it's not whether or not *you* run nntplink 2.1 that determines whether
this will bite you, it's whether or not your neighbor does.

Nntpd is not built to run for a very long time.  Specifically, on
non-USG systems, the history file is only opened once, and is never
closed until nntpd exits.  The problem is that if the history file is
rebuilt (as when expire runs), nntpd does not notice, and continues to
keep the old history file open.  If that old file is deleted, any
running nntpd will hold it open and prevent the disk space from being
deallocated until that nntpd dies thus closing the file.  This hasn't
become such a big problem until recently, with the proliferation of
nntplink and connections that hang around for a long time.

Below are patches to NNTP 1.5.11 to support periodic checking of the
history file by nntpd.  It stat()'s the history file every so often
(default 10 minutes), and if it notices that the file's device id or
inode number changes, as would happen if a new history file were
created, it closes the file.  The file is then reopened the next time
it is needed.

The problem is not new to 1.5.11.  It's just something that
has become an issue recently, so these patches are based on 1.5.11.
Thanks, Stan (or whoever), for the new timer mechanism, upon which this
patch is based.

Steve Groom
stevo@elroy.jpl.nasa.gov

===========================================================================
# This is a shell archive.  Remove anything before this line,
# then unpack it by saving it in a file and typing "sh file".
#
# Wrapped by uniblab!stevo on Mon Feb 11 14:33:33 PST 1991
# Contents:  histcheck.patch
 
echo x - histcheck.patch
sed 's/^@//' > "histcheck.patch" <<'@//E*O*F histcheck.patch//'
*** common/conf.h.dist-1.5.11	Mon Feb 11 12:46:10 1991
--- common/conf.h.dist	Mon Feb 11 13:30:33 1991
***************
*** 170,175 ****
--- 170,184 ----
  #endif ALONE
  
  /*
+  * Enables periodic checking to see if the history file has been rebuilt.
+  * Doesn't apply if USGHIST is used, as history file is not kept open.
+  */
+ 
+ #ifndef USGHIST
+ #define HISTCHECK		(10 * 60)
+ #endif USGHIST
+ 
+ /*
   * Your domain.  This is for the inews generated From: line,
   * assuming that it doesn't find one in the article's head.
   * Suggestions are .UUCP if you don't belong to the Internet.
*** server/common.h-1.5.11	Sat Feb  2 13:55:37 1991
--- server/common.h	Mon Feb 11 12:50:45 1991
***************
*** 97,102 ****
--- 97,108 ----
  #endif
  #endif
  
+ #ifdef HISTCHECK
+ #ifndef TIMERS
+ #define TIMERS
+ #endif
+ #endif
+ 
  /*
   * Some generic maximums.
   */
*** server/misc.c-1.5.11	Tue Dec 11 22:00:11 1990
--- server/misc.c	Mon Feb 11 13:59:13 1991
***************
*** 65,70 ****
--- 65,77 ----
  	return (art_fp);
  }
  
+ /* these are out here so histcheck() can see them */
+ #ifdef DBM
+ static int	dbopen = 0;
+ #else not DBM
+ static DBM	*db = NULL;	/* History file, dbm version */
+ #endif DBM
+ static FILE	*hfp = NULL;	/* history file, text version */
  
  /*
   * gethistent -- return the path name of an article if it's
***************
*** 109,122 ****
  	register int	len;
  #else not USGHIST
  #ifdef DBM
- 	static int	dbopen = 0;
  	datum		fetch();
- #else not DBM
- 	static DBM	*db = NULL;	/* History file, dbm version */
  #endif DBM
  	datum		 key, content;
  #endif USGHIST
- 	static FILE	*hfp = NULL;	/* history file, text version */
  
  #ifdef CNEWS
  	cp = rindex(msg_id,'@');	/* look for @ in message id */
--- 116,125 ----
***************
*** 988,990 ****
--- 991,1058 ----
  }
  #endif
  #endif LOAD
+ 
+ #ifdef HISTCHECK
+ /*
+  * histcheck() - called periodically to determine of history file needs
+  * to be closed and reopened.  Watches the device id and inode number,
+  * if they change the history file must be closed and reopened.
+  *
+  * This is a bit conservative in checking.  If it's already open
+  * the "firsttime" through, there is no uniform way of checking if that
+  * open file is current, so a reset (close/reopen) is forced.
+  */
+ void histcheck() {
+     static int firsttime = 1;
+     static dev_t save_dev;
+     static ino_t save_ino;
+     struct stat sbuf;
+     int res;
+     int isopen;
+ 
+ #ifdef DBM
+     isopen = dbopen;
+ #else not DBM
+     isopen = (db != NULL);
+ #endif DBM
+ 
+     res = stat(historyfile,&sbuf);
+     if (res == -1) {
+ 	firsttime = 1;	/* next time through, assume it's the first */
+ 	if (!isopen) {
+ 	    /* nothing to be done here, let gethistent() worry about it */
+ 	    return;
+ 	}
+     } else if (firsttime) {
+ 	/* stat succeeded, save info for later */
+ 	save_dev = sbuf.st_dev;
+ 	save_ino = sbuf.st_ino;
+     }
+ 
+     /*
+      * If stat failed, assume a reset is called for.
+      * If it's already open the first time through, force a reset
+      * since we don't know for sure that the stat() info matches what's
+      * already open.
+      * Otherwise, reset if dev and/or inode have changed.
+      */
+     if ((firsttime && isopen) || /* open on first check, or stat failed */
+ 	    (bcmp(&save_dev,&sbuf.st_dev,sizeof(dev_t))) || /* dev changed */
+ 	    (bcmp(&save_ino,&sbuf.st_ino,sizeof(ino_t)))) { /* inode changed */
+ #ifdef DBM
+ 	(void) dbmclose();
+ 	dbopen = 0;
+ #else not DBM
+ 	if (db != NULL) {
+ 	    dbm_close(db);
+ 	    db = NULL;
+ 	}
+ #endif DBM
+ 	if (hfp != NULL) {
+ 	    (void) fclose(hfp);
+ 	    hfp = NULL;
+ 	}
+     }
+     firsttime = 0;
+ }
+ #endif HISTCHECK
*** server/serve.c-1.5.11	Thu Jan 10 15:20:08 1991
--- server/serve.c	Mon Feb  4 13:52:20 1991
***************
*** 74,79 ****
--- 74,82 ----
  #ifdef BATCHED_INPUT
  static void batchcheck();
  #endif
+ #ifdef HISTCHECK
+ extern void histcheck();
+ #endif
  
  #ifdef TIMERS
  static struct timer timers[] = {
***************
*** 85,90 ****
--- 88,96 ----
  #endif
  #ifdef BATCHCHECK
  	{ batchcheck, 1, BATCHCHECK, 0 },
+ #endif
+ #ifdef HISTCHECK
+ 	{ histcheck, 0, HISTCHECK, 0 },
  #endif
  };
  #define NTIMERS (sizeof(timers) / sizeof(struct timer))
@//E*O*F histcheck.patch//
chmod u=rw,g=r,o=r histcheck.patch
 
exit 0

-- 
Steve Groom, Jet Propulsion Laboratory, Pasadena, CA
stevo@elroy.jpl.nasa.gov  {ames,usc}!elroy!stevo

alden@shape.mps.ohio-state.edu (Dave Alden) (02/15/91)

In article <1991Feb13.174417.20130@elroy.jpl.nasa.gov> stevo@elroy.jpl.nasa.gov (Steve Groom) writes:
>We ran into some problems with nntpd when neighbors running nntplink
>upgraded to v2.1.  Nntplink is designed to open a connection to a
>remote nntpd and hold it open for a while.  It also contains some code
>to force a disconnection from the remote nntpd after a certain period
>of time.  In v2.1, that code is broken, and nntplink may keep the
>connection open for a very long time (many hours, perhaps days).  A fix
>for nntplink is in the works.

The patch has been placed on shape.mps.ohio-state.edu [128.146.7.200] under
/pub/nntplink/diff-2.1-2.2.shar.Z or you can just pick up the new version
/pub/nntplink/nntplink2.2.tar.Z  -  Please pick up a new copy.  I would
also like to suggest that if you are running nntplink that you get on the
mailing list - there isn't a whole lot of traffic on it however this is
the best way to find out about any bugs that may have been discovered,
along with announcements of bug fixes, new versions, etc...

>In the meantime, I've looked into the
>problem from nntpd's perspective, a sort of defensive move.  You see,
>it's not whether or not *you* run nntplink 2.1 that determines whether
>this will bite you, it's whether or not your neighbor does.

It's probably not a bad idea that this patch adopted as a permanent part
of nntpd.

...dave