This is the mail archive of the
cygwin-apps
mailing list for the Cygwin project.
Fix memleak in setup
- From: Charles Wilson <cygwin at cwilson dot fastmail dot fm>
- To: Mailing List: CygWin-Apps <cygwin-apps at cygwin dot com>
- Date: Sun, 29 Jun 2008 17:28:44 -0400
- Subject: Fix memleak in setup
If compress::decompress returns NULL, we need to clean up the allocated
specialized compress_* objects -- but NOT delete the passed-in
io_stream*. The problem occurs Installer::installOne, but is corrected
by modifying the behavior of compress::decompress and the various
specialized decompression classes.
Detailed explanation:
From Installer::installOne:
if ((try_decompress = compress::decompress (pkgfile)) != NULL)
{
// if we get here, then try_decompress has taken ownership
// of pkgfile
if ((tarstream = archive::extract (try_decompress)) == NULL)
{
// error handling stuff
delete try_decompress;
// since try_decompress owns pkgfile, this cleans
// up both try_decompress and pkgfile.
return;
}
// this is success...tarstream now owns try_decompress,
// which in turn owns pkgfile. Eventually, tarstream
// will be cleaned up along with both try_decompress and
// pkgfile.
}
// however, when the IF statement above fails (returns NULL), then
// without my patch the internal decompress object will NOT be
// deleted before compress::decompress returns NULL. So,
// compress::decompress() needs to delete that object, but since
// we need pkgfile to still be valid in order to do the test below,
// we have to tell the internal decompress object to relinquish
// ownership of pkgfile, and NOT delete it during its own destructor.
// That's why the code here in Installer::installOne is not changed,
// but rather the fix is contained wholly in compress::decompress
// and the specialized decompression classes.
else if ((tarstream = archive::extract (pkgfile)) == NULL)
{
/* Not a compressed tarball, not a plain tarball, give up. */
delete pkgfile;
...
--
Chuck
2008-06-29 Charles Wilson
* compress.cc (compress::decompress): clean up concrete
decompressor objects on failure -- but in that case, do
NOT destroy original io_stream.
* compress_bz.h (compress_bz::release_original): new method.
(owns_original): new member variable.
* compress_bz.cc (compress_bz::release_original): new method.
(compress_bz::compress_bz): take ownership of parent by default.
(compress_bz::~compress_bz): only delete original if
owns_original is true.
* compress_gz.h (compress_gz::release_original): new method.
(owns_original): new member variable.
* compress_gz.cc (compress_gz::release_original): new method.
(compress_gz::construct): take ownership of parent by default.
(compress_gz::~compress_gz): only delete original if
owns_original is true.
Index: compress.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/compress.cc,v
retrieving revision 2.4
diff -u -r2.4 compress.cc
--- compress.cc 27 Dec 2004 14:44:35 -0000 2.4
+++ compress.cc 29 Jun 2008 20:56:42 -0000
@@ -42,6 +42,9 @@
compress_gz *rv = new compress_gz (original);
if (!rv->error ())
return rv;
+ /* else */
+ rv->release_original();
+ delete rv;
return NULL;
}
else if (memcmp (magic, "BZh", 3) == 0)
@@ -49,6 +52,9 @@
compress_bz *rv = new compress_bz (original);
if (!rv->error ())
return rv;
+ /* else */
+ rv->release_original();
+ delete rv;
return NULL;
}
}
Index: compress_bz.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/compress_bz.cc,v
retrieving revision 2.12
diff -u -r2.12 compress_bz.cc
--- compress_bz.cc 30 Jul 2007 22:55:49 -0000 2.12
+++ compress_bz.cc 29 Jun 2008 20:56:43 -0000
@@ -33,6 +33,7 @@
return;
}
original = parent;
+ owns_original = true;
initialisedOk = 0;
bufN = 0;
@@ -202,10 +203,16 @@
return 0;
}
+void
+compress_bz::release_original ()
+{
+ owns_original = false;
+}
+
compress_bz::~compress_bz ()
{
if (initialisedOk)
BZ2_bzDecompressEnd (&strm);
- if (original)
+ if (original && owns_original)
delete original;
}
Index: compress_bz.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/compress_bz.h,v
retrieving revision 2.9
diff -u -r2.9 compress_bz.h
--- compress_bz.h 5 Apr 2005 21:37:41 -0000 2.9
+++ compress_bz.h 29 Jun 2008 20:56:43 -0000
@@ -51,10 +51,12 @@
* over a WAN :} */
virtual size_t get_size () {return 0;};
virtual int get_mtime ();
+ virtual void release_original (); /* give up ownership of original io_stream */
/* if you are still needing these hints... give up now! */
virtual ~ compress_bz ();
private:
io_stream *original;
+ bool owns_original;
char peekbuf[512];
size_t peeklen;
int lasterr;
Index: compress_gz.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/compress_gz.cc,v
retrieving revision 2.12
diff -u -r2.12 compress_gz.cc
--- compress_gz.cc 8 Apr 2008 23:50:54 -0000 2.12
+++ compress_gz.cc 29 Jun 2008 20:56:43 -0000
@@ -54,6 +54,7 @@
compress_gz::construct (io_stream * parent, const char *openmode)
{
original = parent;
+ owns_original = true;
peeklen = 0;
int err;
int level = Z_DEFAULT_COMPRESSION; /* compression level */
@@ -429,6 +430,12 @@
}
void
+compress_gz::release_original ()
+{
+ owns_original = false;
+}
+
+void
compress_gz::destroy ()
{
if (msg)
@@ -450,7 +457,7 @@
free (inbuf);
if (outbuf)
free (outbuf);
- if (original)
+ if (original && owns_original)
delete original;
}
Index: compress_gz.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/compress_gz.h,v
retrieving revision 2.6
diff -u -r2.6 compress_gz.h
--- compress_gz.h 5 Apr 2005 21:37:41 -0000 2.6
+++ compress_gz.h 29 Jun 2008 20:56:43 -0000
@@ -54,6 +54,7 @@
/* Use seek EOF, then tell (). get_size won't do this incase you are sucking down
* over a WAN :} */
virtual size_t get_size () {return 0;};
+ virtual void release_original (); /* give up ownership of original io_stream */
/* if you are still needing these hints... give up now! */
virtual ~ compress_gz ();
private:
@@ -70,6 +71,7 @@
void destroy ();
int do_flush (int);
io_stream *original;
+ bool owns_original;
/* from zlib */
z_stream stream;
int z_err; /* error code for last stream operation */