From: bvs@BitBlocks.com (Bakul Shah)
Subject: Re: Bug in Rlimit initialization? (was Re: getdtablesize)
Organization: Bit Blocks, Inc.
Date: Fri, 2 Oct 1992 19:33:11 GMT

stark@cs.sunysb.edu (Gene Stark) writes:

>I question whether "setrlimit" should be setting the RLIMIT_OFILE and
>RLIMIT_NPROC values to RLIM_INFINITY, rather than NOFILE and MAXUPRC,
>respectively.  The following patch shown below to /sys/kern/init_main.c
>should fix this.  I would appreciate comments on whether this is a
>reasonable thing to do.

>*** /sys/kern/init_main.c	Wed Sep 30 22:30:48 1992
>! 	limit0.pl_rlimit[RLIMIT_OFILE].rlim_cur =
>! 	  limit0.pl_rlimit[RLIMIT_OFILE].rlim_max = NOFILE;

The BSD Net-2 release and 386bsd do not have a fixed limit on how
many open files a user process may have.  If you follow
fd_alloc() in kern/kern_descrip.c, you will notice that the ofile
array (a per process array that keeps track of open files) is
expanded as needed. Your change effectively disables this feature
and puts a fixed limit.  NOFILE is a reasonable default but I'd
like to get as many open files as I can for some applications.

Since the maximum possible number of open file descriptors is now
a function of free space (among others), it is very difficult to
figure out what limit0.pl_rlimit[RLIMIT_OFILE].rlim_max should
be.  We know it will be less than RLIM_INFINITY so setting
rlim_max to that limit is the right thing to do.  Remember, a
limit in this case is an upper bound; you may *never* be able to
reach it.

What is not very smart is allocating as much space as getdtablesize().
What if I do `dup2(0, 123456)'?  This is a nasty but a valid thing
to do given that there is no fixed limit.  As Dave Cline pointed
out in email to me, we need to use sparse array techniques for such
cases.  I've appended popen() diffs at the end that uses linked list
instead of a big fat malloc'ed aray.  I don't know what mtools
uses but the popen() example should point in the right direction.

I still think that for programs that wish to close all open
descriptors, we need something like getdescriptors() (see Antti
Louko's message).

>! 	limit0.pl_rlimit[RLIMIT_NPROC].rlim_cur =
>! 	  limit0.pl_rlimit[RLIMIT_NPROC].rlim_max = MAXUPRC;

Apply similar reasoning as above.

Bakul Shah <bvs@BitBlocks.com>

====================

*** popen.c.dist	Tue Apr 30 17:35:59 1991
--- popen.c	Fri Oct  2 13:30:29 1992
***************
*** 34,39 ****
--- 34,44 ----
   * SUCH DAMAGE.
   */
  
+ /*
+  * Wed Sep 30 19:39:27 PDT 1992 Bakul Shah <bvs@BitBlocks.com>
+  *      Replace the use of getdtablesize() size array with a linked list.
+  */
+ 
  #if defined(LIBC_SCCS) && !defined(lint)
  static char sccsid[] = "@(#)popen.c	5.15 (Berkeley) 2/23/91";
  #endif /* LIBC_SCCS and not lint */
***************
*** 48,54 ****
  #include <unistd.h>
  #include <paths.h>
  
! static pid_t *pids;
  
  FILE *
  popen(program, type)
--- 53,66 ----
  #include <unistd.h>
  #include <paths.h>
  
! struct fdpid {
! 	struct fdpid *  next;
! 	int             fd;
! 	pid_t           pid;
! };
! 
! static struct fdpid *fdpids;
! static struct fdpid ** lastfdpidp = &fdpids;
  
  FILE *
  popen(program, type)
***************
*** 57,79 ****
  {
  	FILE *iop;
  	int pdes[2], fds, pid;
  
  	if (*type != 'r' && *type != 'w' || type[1])
  		return (NULL);
  
- 	if (pids == NULL) {
- 		if ((fds = getdtablesize()) <= 0)
- 			return (NULL);
- 		if ((pids = (pid_t *)malloc((u_int)(fds * sizeof(int)))) == NULL)
- 			return (NULL);
- 		bzero((char *)pids, fds * sizeof(pid_t));
- 	}
  	if (pipe(pdes) < 0)
  		return (NULL);
  	switch (pid = vfork()) {
  	case -1:			/* error */
  		(void) close(pdes[0]);
  		(void) close(pdes[1]);
  		return (NULL);
  		/* NOTREACHED */
  	case 0:				/* child */
--- 69,89 ----
  {
  	FILE *iop;
  	int pdes[2], fds, pid;
+ 	struct fdpid * fp;
  
  	if (*type != 'r' && *type != 'w' || type[1])
  		return (NULL);
  
  	if (pipe(pdes) < 0)
  		return (NULL);
+ 	fp = (struct fdpid *)malloc(sizeof *fp);
+ 	if (fp == NULL)
+ 		return NULL;
  	switch (pid = vfork()) {
  	case -1:			/* error */
  		(void) close(pdes[0]);
  		(void) close(pdes[1]);
+ 		free(fp);
  		return (NULL);
  		/* NOTREACHED */
  	case 0:				/* child */
***************
*** 102,108 ****
  		iop = fdopen(pdes[1], type);
  		(void) close(pdes[0]);
  	}
! 	pids[fileno(iop)] = pid;
  	return (iop);
  }
  
--- 112,122 ----
  		iop = fdopen(pdes[1], type);
  		(void) close(pdes[0]);
  	}
! 	fp->pid = pid;
! 	fp->fd = fileno(iop);
! 	fp->next = NULL;
! 	*lastfdpidp = fp;
! 	lastfdpidp = &fp->next;
  	return (iop);
  }
  
***************
*** 114,119 ****
--- 128,135 ----
  	int omask;
  	union wait pstat;
  	pid_t pid;
+ 	struct fdpid * fp;
+ 	struct fdpid ** fpp;
  
  	/*
  	 * pclose returns -1 if stream is not associated with a
***************
*** 120,133 ****
  	 * `popened' command, if already `pclosed', or waitpid
  	 * returns an error.
  	 */
! 	if (pids == NULL || pids[fdes = fileno(iop)] == 0)
  		return (-1);
  	(void) fclose(iop);
  	omask = sigblock(sigmask(SIGINT)|sigmask(SIGQUIT)|sigmask(SIGHUP));
  	do {
! 		pid = waitpid(pids[fdes], (int *) &pstat, 0);
  	} while (pid == -1 && errno == EINTR);
  	(void) sigsetmask(omask);
! 	pids[fdes] = 0;
  	return (pid == -1 ? -1 : pstat.w_status);
  }
--- 136,155 ----
  	 * `popened' command, if already `pclosed', or waitpid
  	 * returns an error.
  	 */
! 	fdes = fileno(iop);
! 	for (fpp = &fdpids; (fp = *fpp) != NULL; fpp = &fp->next)
! 		if (fp->fd == fdes)
! 			break;
! 	if (fp == NULL)
  		return (-1);
  	(void) fclose(iop);
  	omask = sigblock(sigmask(SIGINT)|sigmask(SIGQUIT)|sigmask(SIGHUP));
  	do {
! 		pid = waitpid(fp->pid, (int *) &pstat, 0);
  	} while (pid == -1 && errno == EINTR);
  	(void) sigsetmask(omask);
! 	if ((*fpp = fp->next) == NULL)
! 		lastfdpidp = fpp;
! 	free(fp);
  	return (pid == -1 ? -1 : pstat.w_status);
  }