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>
>