Hi,
I like this approach. I have a number of often spawned daemon
processes that could benefit from this. One of the last process
we debugged where we had unwanted open filedescriptors was in
programs invoked by the cvs loginfo script.
For naming convention considerations, I might suggest 'closeall'
or 'closefdset' or something similar... at least have 'close' in
name... :-)
Good Work,
John
> It's fairly common, when spawning new processes, to want to make sure
> all unwanted FDs are closed. Currently, the options for doing this are:
>
> 1) Use fcntl(fd, F_SETFD, FD_CLOEXEC) to set the close-on-exec flag
> when the file is opened/cloned. This may not be practical if the
> FD must remain open across some exec's, but not others. It is not
> possible to ensure that FDs implicitly opened within library
> functions have the close-on-exec flag set. It may be inefficient if
> there are lots of opens and few execs.
>
> 2) Explicitly close unwanted FDs in the child, before the exec().
> This suffers from the usual resource tracking problems (ie it's
> easy to forget to close one - especially in a maze of pipe()/
> dup()/dup2() calls designed to join the child's stdin/out/err
> to the parent). It also requires that the FD's be visible to the
> function - which may be difficult (the FD may be hidden within
> another function somewhere).
>
> 3) Close all FDs except the ones you explicitly want to keep. This
> is normally something like:
> for (i = getdtablesize(); --i > 2; )
> close(i);
> The advantage is that you are sure you don't miss any. The
> disadvantage is that it requires a system call for each potentially
> open FD - >600 on my system - whereas maybe only 4 or 5 are
> actually open.
>
> In the case of option 3, you only really need to attempt to close file
> descriptors less then curproc->p_fd->fd_lastfile or even
> curproc->p_fd->fd_nfiles, but these values aren't readily accessible
> from userland. (And this still suffers the overhead of a userland
> to kernel transition for each FD).
>
> I'd therefore like to propose a new syscall that closes _all_ file
> descriptors associated with a process, except for those passed as
> 1 bits in an fd_set. The proposed API is:
>
> int cleanup_files(int nfds, const fd_set *leavefds);
>
> where nfds specifies the number of fds in *leavefds to potentially
> keep open (ie all fds >= nfds will be closed).
>
> The function would return the number of FDs closed.
>
> The implementation would be along the lines of:
>
> struct cleanup_files_args {
> int nd;
> fd_set *leave;
> };
>
> int
> cleanup_files(p, uap)
> register struct proc *p;
> register struct cleanup_files_args *uap;
> {
> fd_set s_fdset;
> fd_set *lset;
> int error, nclosed = 0;
> u_int i, ncpbytes, nfdbits;
> struct filedesc *fdp = p->p_fd;
> struct file **fpp;
> char *fdfp;
>
> if (uap->nd < 0 || uap->leave = NULL)
> return (EINVAL);
>
> /* some daemons mught not have any file descriptors */
> if (fdp == NULL) {
> p->p_retval[0] = 0;
> return (0);
> }
>
> if (uap->nd > fdp->fd_lastfile)
> uap->nd = fdp->fd_lastfile + 1;
>
> /*
> * Allocate just enough bits for the passed fd_set. Use the
> * preallocated auto buffer if possible.
> */
> nfdbits = roundup(uap->nd, NFDBITS);
> ncpbytes = nfdbits / NBBY;
> if (ncpbytes <= sizeof s_fdset)
> lset = &s_fdset;
> else
> lset = malloc(ncpbytes, M_SELECT, M_WAITOK);
>
> error = copyin(uap->leave, lbits, ncpbytes);
> if (error != 0)
> goto done;
>
> fpp = fdp->fd_ofiles;
> fdfp = fdp->fd_ofileflags;
> for (i = 0; i < uap->nd; i++, fpp++, fdfp++) {
> if (!FD_ISSET(i, lset) && *fpp != NULL) {
> if (*fdfp & UF_MAPPED)
> (void) munmapfd(p, i);
> error = closef(*fpp, p);
> if (error != 0)
> goto done;
> nclosed++;
> *fpp = NULL;
> *fdfp = 0;
> if (i < fdp->fd_freefile)
> fdp->fd_freefile = i;
> }
> }
>
> for (; i <= fdp->fd_lastfile; i++, fpp++, fdfp++)
> if (*fpp != NULL) {
> if (*fdfp & UF_MAPPED)
> (void) munmapfd(p, i);
> error = closef(*fpp, p);
> if (error != 0)
> goto done;
> nclosed++;
> *fpp = NULL;
> *fdfp = 0;
> if (i < fdp->fd_freefile)
> fdp->fd_freefile = i;
> }
> }
>
> while (fdp->fd_lastfile > 0 && fdp->fd_ofiles[fdp->fd_lastfile] == NULL)
> fdp->fd_lastfile--;
>
> done:
> if (lset != &s_fdset)
> free(lset, M_SELECT);
>
> p->p_retval[0] = nclosed;
> return (error);
> }
>
>
> Peter
>
>
> To Unsubscribe: send mail to [EMAIL PROTECTED]
> with "unsubscribe freebsd-hackers" in the body of the message
>
> ------------------------------
>
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
