Re: Memory leaks in apt
Hello all,
Hi Julian Andres Klode,
(btw: It is not only _config - _error is basicly the same)
i am maybe wrong, but:
The memory is freed by the operation system anyway after the program exits
and both variables are (possibly) used till the end of the program,
so a free/delete just before the final return seems useless to me.
Also to really fix it, we have to free the two variables on all possible
exit points in the program - this could be simplified with a registered
atexit() method, even included in the library for all programs,
but again for me it feels a bit redundant to do all this work just to free
some memory which would be freed by the operation system a few microseconds
later anyway. (I know that this slant could lead to a "APT is not a deamon
so it will exit in a short while anyway -> why fixing any memory leak?" -
but this would be really bad style imo)
This said, it is maybe a good idea to add both deletes at the very end of
a normal run just to please valgrind and to be able to say in (a far away)
future that APT is totally memory leak free. ;)
And to finally add some usefulness to this email:
The attached patch should fix some (very) minor¹ leaks by adding a few
destructors to the classes debReleaseIndex, debSrcRecordParser, metaIndex
and CacheFile (the first three are in apt-pkg [ABI], the last in apt-get)
Best regards / Mit freundlichen Grüßen,
David "DonKult" Kalnischkies
¹ I test it with a simulation of a ~200 package dist-upgrade:
less than 100k bytes saved...
=== modified file 'apt-pkg/deb/debmetaindex.cc'
--- apt-pkg/deb/debmetaindex.cc 2008-11-16 13:58:02 +0000
+++ apt-pkg/deb/debmetaindex.cc 2009-06-16 13:16:15 +0000
@@ -115,6 +115,13 @@
this->Type = "deb";
}
+debReleaseIndex::~debReleaseIndex()
+{
+ for (vector<const debSectionEntry *>::const_iterator I = SectionEntries.begin();
+ I != SectionEntries.end(); I++)
+ delete *I;
+}
+
vector <struct IndexTarget *>* debReleaseIndex::ComputeIndexTargets() const
{
vector <struct IndexTarget *>* IndexTargets = new vector <IndexTarget *>;
=== modified file 'apt-pkg/deb/debmetaindex.h'
--- apt-pkg/deb/debmetaindex.h 2006-12-14 11:39:29 +0000
+++ apt-pkg/deb/debmetaindex.h 2009-06-16 13:16:15 +0000
@@ -22,6 +22,7 @@
public:
debReleaseIndex(string URI, string Dist);
+ ~debReleaseIndex();
virtual string ArchiveURI(string File) const {return URI + File;};
virtual bool GetIndexes(pkgAcquire *Owner, bool GetAll=false) const;
=== modified file 'apt-pkg/deb/debsrcrecords.cc'
--- apt-pkg/deb/debsrcrecords.cc 2008-11-16 13:58:02 +0000
+++ apt-pkg/deb/debsrcrecords.cc 2009-06-16 13:16:15 +0000
@@ -152,3 +152,11 @@
return true;
}
/*}}}*/
+// SrcRecordParser::~SrcRecordParser - Destructor /*{{{*/
+// ---------------------------------------------------------------------
+/* */
+debSrcRecordParser::~debSrcRecordParser()
+{
+ delete[] Buffer;
+}
+ /*}}}*/
=== modified file 'apt-pkg/deb/debsrcrecords.h'
--- apt-pkg/deb/debsrcrecords.h 2008-11-16 13:58:02 +0000
+++ apt-pkg/deb/debsrcrecords.h 2009-06-16 13:16:15 +0000
@@ -50,6 +50,7 @@
debSrcRecordParser(string File,pkgIndexFile const *Index)
: Parser(Index), Fd(File,FileFd::ReadOnly), Tags(&Fd,102400),
Buffer(0), BufSize(0) {}
+ ~debSrcRecordParser();
};
#endif
=== modified file 'apt-pkg/metaindex.h'
--- apt-pkg/metaindex.h 2006-12-14 11:39:29 +0000
+++ apt-pkg/metaindex.h 2009-06-16 13:16:15 +0000
@@ -39,7 +39,11 @@
virtual vector<pkgIndexFile *> *GetIndexFiles() = 0;
virtual bool IsTrusted() const = 0;
- virtual ~metaIndex() {};
+ virtual ~metaIndex() {
+ for (vector<pkgIndexFile *>::iterator I = (*Indexes).begin(); I != (*Indexes).end(); ++I)
+ delete *I;
+ delete Indexes;
+ }
};
#endif
=== modified file 'cmdline/apt-get.cc'
--- cmdline/apt-get.cc 2009-06-13 16:00:57 +0000
+++ cmdline/apt-get.cc 2009-06-16 13:16:15 +0000
@@ -111,6 +111,9 @@
return Open(true);
}
CacheFile() : List(0) {};
+ ~CacheFile() {
+ delete[] List;
+ }
};
/*}}}*/
// IsAutoInstallOk - implements a few MarkInstall package checks /*{{{*/
Reply to: