Bug 11691 - stdio_filebuf leaks FILE buffer when "no close" is requested
Summary: stdio_filebuf leaks FILE buffer when "no close" is requested
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 3.3
: P2 normal
Target Milestone: 3.4.0
Assignee: Benjamin Kosnik
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-07-28 04:04 UTC by Jeff B
Modified: 2003-12-07 06:17 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2003-07-31 03:50:39


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jeff B 2003-07-28 04:04:29 UTC
This bug (?) can be re-created on Red Hat Linux 9.0
runnig GNUC > 3.


If I compile the code below (must be GNUC > 3), run it and
monitor the VmSize shown using /proc/<pid>/status, it
increases by 4kB every time through the loop.

I took a look at the source and it appears that fdopen is
called in __basic_file (with the file descriptor passed in,
stdout here for test purposes) and the resulting FILE* is
assigned to private variable _M_cfile.

If the third parameter in the stdio_filebuf ctor is set to
false, fclose is never called for _M_cfile, but the
FILE buffer/stream (_pointed to by _M_cfile) opened using
fdopen, isn't accessible, so the buffer is never freed.

It seems to me that if I pass in a fd and tell stdio_filebuf
not to close the file, it would still close the FILE buffer
or at least provide some way for me to access it so I can
call fclose when I'm done.  Since all I have is the fd
I passed in, all I can do is call close() ... which results
in the leak I mentioned.

Note that this was originally discover using fd's from a
pipe() call, I just used stdout to simplify the example.



#include <unistd.h>
#include <iostream>
#include <ext/stdio_filebuf.h>

int main()
{
  std::cout << "pid = " << getpid() << std::endl;


  for (int i=0; i<50; ++i)
  {
     __gnu_cxx::stdio_filebuf<char> filebuf(1,std::ios_base::out,false,1024);
     sleep(1);
  }
}
Comment 1 Andrew Pinski 2003-07-31 03:50:38 UTC
I can confirm the leak in the mainline (20030730), it is bad leak as if you remove the limit 
on the for loop and the sleep, the program leaks memory fast.
Comment 2 Benjamin Kosnik 2003-12-05 23:57:16 UTC
Ack. I added comments to the wrong bug. Let's try again:

----

I think the bool argument can just be removed: it was put in to deal with the
std streams (cin, cout, etc), and don't really apply to people using
stdio_filebuf for other work.

Or, I can keep the argument but have it do nothing (less impressive, but keeps
the same API). Here's a patch for the first idea, please let me know what you think.

best,
benjamin

2003-12-05  Benjamin Kosnik  <bkoz@redhat.com>

	PR libstdc++/11691 
	* include/ext/stdio_filebuf.h (stdio_filebuf::stdio_filebuf):
	Remove __del argument to file descriptor constructor.
	* config/io/basic_file_stdio.h (__basic_file::sys_open): Remove
	bool argument.
	* config/io/basic_file_stdio.cc: Same.

Index: config/io/basic_file_stdio.cc
===================================================================
RCS file: /cvs/gcc/gcc/libstdc++-v3/config/io/basic_file_stdio.cc,v
retrieving revision 1.23
diff -c -p -r1.23 basic_file_stdio.cc
*** config/io/basic_file_stdio.cc	22 Oct 2003 15:51:55 -0000	1.23
--- config/io/basic_file_stdio.cc	5 Dec 2003 23:43:21 -0000
*************** namespace std 
*** 140,147 ****
    }
    
    __basic_file<char>*
!   __basic_file<char>::sys_open(int __fd, ios_base::openmode __mode, 
! 			       bool __del) 
    {
      __basic_file* __ret = NULL;
      int __p_mode = 0;
--- 140,146 ----
    }
    
    __basic_file<char>*
!   __basic_file<char>::sys_open(int __fd, ios_base::openmode __mode)
    {
      __basic_file* __ret = NULL;
      int __p_mode = 0;
*************** namespace std 
*** 151,162 ****
      _M_open_mode(__mode, __p_mode, __rw_mode, __c_mode);
      if (!this->is_open() && (_M_cfile = fdopen(__fd, __c_mode)))
        {
! 	// Iff __del is true, then close will fclose the fd.
! 	_M_cfile_created = __del;
! 
  	if (__fd == 0)
  	  setvbuf(_M_cfile, reinterpret_cast<char*>(NULL), _IONBF, 0);
- 
  	__ret = this;
        }
      return __ret;
--- 150,158 ----
      _M_open_mode(__mode, __p_mode, __rw_mode, __c_mode);
      if (!this->is_open() && (_M_cfile = fdopen(__fd, __c_mode)))
        {
! 	_M_cfile_created = true;
  	if (__fd == 0)
  	  setvbuf(_M_cfile, reinterpret_cast<char*>(NULL), _IONBF, 0);
  	__ret = this;
        }
      return __ret;
*************** namespace std 
*** 199,205 ****
    __basic_file<char>* 
    __basic_file<char>::close()
    { 
!     __basic_file* __retval = static_cast<__basic_file*>(NULL);
      if (this->is_open())
        {
  	if (_M_cfile_created)
--- 195,201 ----
    __basic_file<char>* 
    __basic_file<char>::close()
    { 
!     __basic_file* __ret = static_cast<__basic_file*>(NULL);
      if (this->is_open())
        {
  	if (_M_cfile_created)
*************** namespace std 
*** 207,215 ****
  	else
  	  fflush(_M_cfile);
  	_M_cfile = 0;
! 	__retval = this;
        }
!     return __retval;
    }
   
    streamsize 
--- 203,211 ----
  	else
  	  fflush(_M_cfile);
  	_M_cfile = 0;
! 	__ret = this;
        }
!     return __ret;
    }
   
    streamsize 
Index: config/io/basic_file_stdio.h
===================================================================
RCS file: /cvs/gcc/gcc/libstdc++-v3/config/io/basic_file_stdio.h,v
retrieving revision 1.15
diff -c -p -r1.15 basic_file_stdio.h
*** config/io/basic_file_stdio.h	16 Oct 2003 22:37:48 -0000	1.15
--- config/io/basic_file_stdio.h	5 Dec 2003 23:43:21 -0000
***************
*** 1,6 ****
  // Wrapper of C-language FILE struct -*- C++ -*-
  
! // Copyright (C) 2000, 2001, 2002 Free Software Foundation, Inc.
  //
  // This file is part of the GNU ISO C++ Library.  This library is free
  // software; you can redistribute it and/or modify it under the
--- 1,6 ----
  // Wrapper of C-language FILE struct -*- C++ -*-
  
! // Copyright (C) 2000, 2001, 2002, 2003 Free Software Foundation, Inc.
  //
  // This file is part of the GNU ISO C++ Library.  This library is free
  // software; you can redistribute it and/or modify it under the
*************** namespace std 
*** 74,80 ****
        sys_open(__c_file* __file, ios_base::openmode);
  
        __basic_file*
!       sys_open(int __fd, ios_base::openmode __mode, bool __del);
  
        __basic_file* 
        close(); 
--- 74,80 ----
        sys_open(__c_file* __file, ios_base::openmode);
  
        __basic_file*
!       sys_open(int __fd, ios_base::openmode __mode);
  
        __basic_file* 
        close(); 
Index: include/ext/stdio_filebuf.h
===================================================================
RCS file: /cvs/gcc/gcc/libstdc++-v3/include/ext/stdio_filebuf.h,v
retrieving revision 1.14
diff -c -p -r1.14 stdio_filebuf.h
*** include/ext/stdio_filebuf.h	12 Nov 2003 01:14:34 -0000	1.14
--- include/ext/stdio_filebuf.h	5 Dec 2003 23:43:22 -0000
*************** namespace __gnu_cxx
*** 65,78 ****
        /**
         *  @param  fd  An open file descriptor.
         *  @param  mode  Same meaning as in a standard filebuf.
-        *  @param  del  Whether to close the file on destruction.
         *  @param  size  Optimal or preferred size of internal buffer, in chars.
         *
         *  This constructor associates a file stream buffer with an open
!        *  POSIX file descriptor.  Iff @a del is true, then the associated
!        *  file will be closed when the stdio_filebuf is closed/destroyed.
        */
!       stdio_filebuf(int __fd, std::ios_base::openmode __mode, bool __del, 
  		    size_t __size = static_cast<size_t>(BUFSIZ));
  
        /**
--- 65,76 ----
        /**
         *  @param  fd  An open file descriptor.
         *  @param  mode  Same meaning as in a standard filebuf.
         *  @param  size  Optimal or preferred size of internal buffer, in chars.
         *
         *  This constructor associates a file stream buffer with an open
!        *  POSIX file descriptor.
        */
!       stdio_filebuf(int __fd, std::ios_base::openmode __mode, 
  		    size_t __size = static_cast<size_t>(BUFSIZ));
  
        /**
*************** namespace __gnu_cxx
*** 114,123 ****
  
    template<typename _CharT, typename _Traits>
      stdio_filebuf<_CharT, _Traits>::
!     stdio_filebuf(int __fd, std::ios_base::openmode __mode, bool __del, 
! 		  size_t __size)
      {
!       this->_M_file.sys_open(__fd, __mode, __del);
        if (this->is_open())
  	{
  	  this->_M_mode = __mode;
--- 112,120 ----
  
    template<typename _CharT, typename _Traits>
      stdio_filebuf<_CharT, _Traits>::
!     stdio_filebuf(int __fd, std::ios_base::openmode __mode, size_t __size)
      {
!       this->_M_file.sys_open(__fd, __mode);
        if (this->is_open())
  	{
  	  this->_M_mode = __mode;


Comment 3 CVS Commits 2003-12-07 03:46:18 UTC
Subject: Bug 11691

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	bkoz@gcc.gnu.org	2003-12-07 03:46:14

Modified files:
	libstdc++-v3   : ChangeLog 
	libstdc++-v3/config/io: basic_file_stdio.cc basic_file_stdio.h 
	libstdc++-v3/include/ext: stdio_filebuf.h 

Log message:
	2003-12-06  Benjamin Kosnik  <bkoz@redhat.com>
	
	PR libstdc++/11691
	* include/ext/stdio_filebuf.h (stdio_filebuf::stdio_filebuf):
	Remove __del argument to file descriptor constructor.
	* config/io/basic_file_stdio.h (__basic_file::sys_open): Remove
	bool argument.
	* config/io/basic_file_stdio.cc: Same.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&r1=1.2131&r2=1.2132
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/config/io/basic_file_stdio.cc.diff?cvsroot=gcc&r1=1.23&r2=1.24
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/config/io/basic_file_stdio.h.diff?cvsroot=gcc&r1=1.15&r2=1.16
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/ext/stdio_filebuf.h.diff?cvsroot=gcc&r1=1.14&r2=1.15

Comment 4 Andrew Pinski 2003-12-07 06:17:08 UTC
Fixed for 3.4.
Comment 5 CVS Commits 2004-05-19 08:31:16 UTC
Subject: Bug 11691

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	redi@gcc.gnu.org	2004-05-19 08:31:07

Modified files:
	libstdc++-v3   : ChangeLog 
	libstdc++-v3/include/ext: stdio_filebuf.h 

Log message:
	2004-05-18  Jonathan Wakely  <redi@gcc.gnu.org>
	
	* include/ext/stdio_filebuf.h: Update comments to reflect PR 11691.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&r1=1.2483&r2=1.2484
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/ext/stdio_filebuf.h.diff?cvsroot=gcc&r1=1.16&r2=1.17

Comment 6 CVS Commits 2004-05-19 09:51:49 UTC
Subject: Bug 11691

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	redi@gcc.gnu.org	2004-05-19 09:51:36

Modified files:
	libstdc++-v3   : ChangeLog 
	libstdc++-v3/include/ext: stdio_filebuf.h 

Log message:
	2004-05-18  Jonathan Wakely  <redi@gcc.gnu.org>
	
	* include/ext/stdio_filebuf.h: Update comments to reflect PR 11691.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.2224.2.107&r2=1.2224.2.108
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/ext/stdio_filebuf.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.16&r2=1.16.4.1