www.delorie.com/djgpp/bugs/show.cgi   search  
Bug 000062

When Created: 02/22/1996 08:25:08
Against DJGPP version: 2.00
By whom: Broeker@axpmgr.physik.rwth-aachen.de
Abstract: popen()/pclose() seems to lose file handles
While using info.exe with lots of the info-files being gzipped, I had to
find out that after the 6th or so of them being opened, info reported
that it couldn't open some files (go32.exe, or info.exe mostly), and
from then on, no gzipped info file could be read any more.

Inspection in the debugger revealed that the file handle numbers in the
FILE *'s returned by the popen() calls that call gunzip.exe to do the
decompression kept increasing by 2 each time, until they reached the 
magical value of 19. The text read from the popen()'d file with handle 19 
then only contained an error message in the style of 'could not open file'.

So, as it seems, the popen()/pclose() pair of routines loses 2 file handles
for every call. Once the usual 20 handles are used up, the program stops
working correctly.

This also seems to cause a failure in the distributed version of patch.exe:
Wnen patching several files by calling 'patch <some.pat' or so, it works
correctly for the first ones, but then suddendly stops with an error
message like 'couldn't open file for writing'.

I'm going to get the libsources to find out if I can fix this.

Note added: 03/04/1996 08:48:20
By whom: Broeker@axpmgr.physik.rwth-aachen.de
A small addition: The same problem seems to occur with the system()
call as well, but only if the programs called are DJGPP-compiled ones,
and they contain I/O redirection. In that case, system() tries to
handle the redirection on its own, acting almost exactly like
popen() does, and making, apparently, the same errors as well.

Demonstration case: repeated calling of

system("gunzip <foo.txt");	-- fails after about 12 calls
system("gzcat foo.txt");	-- doesn't fail 

Solution added: 03/04/1996 09:11:49
By whom: Broeker@axpmgr.physik.rwth-aachen.de
This is my fix for the bug I reported last week (Fri, 22. Feb 1996).
After some digging around, it turned out that the problem was indeed
caused by a loss of file handles: as far as I can see, the original
problem (info stops working after reading about 6 or 7 compressed files)
can be fixed by following the call sequence

   int handle=dup(fileno(stdout))
   FILE *outfile=freopen("file.out","wb",stdout)
   ..
   ..
   ..
   fclose(outfile);
   dup2(handle, fileno(stdout));

by a line like this:

   close(handle);

(Detailed patches are included at the end of this message)

I do not quite understand why this problem didn't show up with V1.12: the
sources for popen() have hardly changed at all, and there even was one
close() call less than there is now (only executed if some error
condition was hit). Maybe dup() or dup2() was changed? If so, which
version is correct: V1 or V2?

Another hint: maybe we should add functionality to increase the number
of possible file handles beyond the normal 20? (DOS function 0x67 could
come in handy here...)

In addition to popen()/pclose(), I also found the same kind of
misbehaviour in system(). Patches for both popen.c and system.c are
appended here: 

--- system.c    Sat Nov 25 02:31:26 1995
+++ systemhb.c  Sat Mar  2 19:34:24 1996
@@ -253,11 +253,13 @@
          {
            dup2 (h_inbak, 0);
            close (h_in);
+           close (h_inbak);  /* HBB: without, loses handles */
          }
          if (f_out)
          {
            dup2 (h_outbak, 1);
            close (h_out);
+           close (h_outbak); /* HBB: dito */
          }
          s = u;
          break;

--- popen.c     Sun Nov 19 06:27:16 1995
+++ popen_hb.c  Sat Mar  2 20:06:52 1996
@@ -119,21 +119,20 @@
       /* dup stdout */
       if ((l1->fd = dup (fileno (stdout))) == EOF)
        l1->fp = NULL;
-      else if (!(l1->fp = freopen (l1->temp_name, "wb", stdout)))
-       l1->fp = NULL;
-      else
+/* HBB: original version was kind of double-done */
+         else if (l1->fp = freopen (l1->temp_name, "wb", stdout))
        /* exec cmd */
-       if (system (cm) == EOF)
+/* HBB: better, as system() isn't guaranteed to return EOF on error */
+           if (system (cm))
          l1->fp = NULL;
       /* reopen real stdout */
       if (dup2 (l1->fd, fileno (stdout)) == EOF)
-      {
        l1->fp = NULL;
-       close(l1->fd);
-      }
       else
        /* open file for reader */
        l1->fp = fopen (l1->temp_name, l1->mode);
+/* HBB: this ensures l1->fd is always closed. Fix for 'lost file handles' bug */
+         close (l1->fd);
     }
     else
       /* if caller wants to write */
@@ -200,8 +199,9 @@
             /* reopen stdin */
            if (dup2 (l1->fd, fileno (stdin)) == EOF)
              retval = -1;
-           close(l1->fd);
          }
+/* HBB: ensure l1->fd is always closed */
+          close (l1->fd);
     }
     else
       /* if pipe was opened to read */

Fixed in version 2.01 on 06/12/1996 23:56:27
By whom: dj@delorie.com



  webmaster   donations   bookstore     delorie software   privacy  
  Copyright 2010   by DJ Delorie     Updated Jul 2010