Subject: lost file descriptors in Squid on NetBSD-1.3.3 apparently solved, but with a different patch....
To: NetBSD Networking Technical Discussion List <tech-net@NetBSD.ORG>
From: Greg A. Woods <woods@most.weird.com>
List: tech-net
Date: 10/26/1999 14:25:59
It would appear (from two "panic: closef: count < 0" failures in less
than 12 hours) that Darren's fix to accept(2) for lost file descriptors
isn't quite correct.  His fix inserts a call to closef() to handle one
of several possible error conditions.  However everywhere else in the
socket code in the same file where falloc() cleanup is necessary the
function used is ffree().

Other than the occasional panic, Darren's fix does seem to eliminate the
problem of lost file descriptors in Squid ;-)

I do note that there may be other changes in -current that may have
avoided the panic(), but none the less I believe the correct cleanup
function to use is ffree().

In fact on further re-examination of the code I see a second place where
it seems to me that an error condition should definitely not be ignored,
i.e. in the call to soaccept().  It would seem to me to be much more
appropriate to detect these errors in accept() rather than waiting for a
read() or write() call to detect them.

I've wondered about both of these situations ever since I first read
this part of the code, and indeed examination of 4.4BSD-Lite2 and
Stevens' "TCP/IP Illustrated" did not satisfy my concerns as no
explanation was given for why various errors were ignored.

I believe the following patch will fully correct the problem.  I've been
running it in NetBSD-1.3.3 for a few hours today with a rather busy (up
to 15 reqests/s) squid server to great success, and indeed the
symptomatic error messages that previously indicated lost file
descriptors are not causing lost file descriptors, and so far (knock on
wood) there's been no further panic.

Diff is from:
/*      $NetBSD: uipc_syscalls.c,v 1.45 1999/07/01 08:12:47 itojun Exp $        


*** /most/var/sup/sup.NetBSD.ORG/src/sys/kern/uipc_syscalls.c	Thu Jul  1 07:31:16 1999
--- /usr/src/sys/kern/uipc_syscalls.c	Tue Oct 26 12:21:49 1999
***************
*** 244,265 ****
  	fp->f_data = (caddr_t)so;
  	FILE_UNUSE(fp, p);
  	nam = m_get(M_WAIT, MT_SONAME);
! 	(void) soaccept(so, nam);
  	if (SCARG(uap, name)) {
  		if (namelen > nam->m_len)
  			namelen = nam->m_len;
  		/* SHOULD COPY OUT A CHAIN HERE */
  		if ((error = copyout(mtod(nam, caddr_t),
! 		    (caddr_t)SCARG(uap, name), namelen)) == 0)
! 			error = copyout((caddr_t)&namelen,
! 			    (caddr_t)SCARG(uap, anamelen),
! 			    sizeof(*SCARG(uap, anamelen)));
! 		if (error != 0)
! 			(void) closef(fp, p);
  	}
  	m_freem(nam);
  	splx(s);
  	return (error);
  }
  
  /* ARGSUSED */
--- 244,274 ----
  	fp->f_data = (caddr_t)so;
  	FILE_UNUSE(fp, p);
  	nam = m_get(M_WAIT, MT_SONAME);
! 	if ((error = soaccept(so, nam)))
! 		goto freeit;
  	if (SCARG(uap, name)) {
  		if (namelen > nam->m_len)
  			namelen = nam->m_len;
  		/* SHOULD COPY OUT A CHAIN HERE */
  		if ((error = copyout(mtod(nam, caddr_t),
! 				     (caddr_t)SCARG(uap, name),
! 				     namelen)))
! 			goto freeit;
! 		if ((error = copyout((caddr_t)&namelen,
! 				     (caddr_t)SCARG(uap, anamelen),
! 				     sizeof(*SCARG(uap, anamelen)))))
! 			goto freeit;
  	}
  	m_freem(nam);
  	splx(s);
  	return (error);
+ 
+  freeit:
+ 	ffree(fp);
+ 	m_freem(nam);
+ 	splx(s);
+ 	return (error);
+ 
  }
  
  /* ARGSUSED */


Shall I send-pr this change?

-- 
							Greg A. Woods

+1 416 218-0098      VE3TCP      <gwoods@acm.org>      <robohack!woods>
Planix, Inc. <woods@planix.com>; Secrets of the Weird <woods@weird.com>