Subject: Re: lost file descriptors in Squid on NetBSD-1.3.3 apparently solved, but with a different patch....
To: None <tech-net@netbsd.org>
From: Darren Reed <darrenr@reed.wattle.id.au>
List: tech-net
Date: 10/27/1999 21:55:22
Patch applied.  uipc_syscalls.c Rev. 1.46.

Darren

In some email I received from Greg A. Woods, sie wrote:
> 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>
>