Thread: libpq++ memory problems
I have written the following small piece of code to illustrate my
problem.
#include <pgsql/libpq++.h>
void callproc(){ PgDatabase* loStatsdb; PgEnv loEnv("","","","","");
loStatsdb = new PgDatabase(loEnv,"ftldb"); delete loStatsdb;
};
main(){ while(1){ callproc(); };
};
The procedure 'callproc' is called in a continuous loop to emphasise
this problem. Within the procedure all that happens is a new database
connection is made and deleted. On my system this eats memory at an
enormous rate.
I do not know if this problem has been discussed before or fixed in
newer versions of the library (my apologies if it has), but I would be
most grateful of any info on resolving this memory leak.
Tim Brookes
Tim Brookes <tim.brookes@mcmail.com> writes:
> The procedure 'callproc' is called in a continuous loop to emphasise
> this problem. Within the procedure all that happens is a new database
> connection is made and deleted. On my system this eats memory at an
> enormous rate.
I could not reproduce such a leak with current sources.
It appears that you are using a fairly old Postgres release --- class
PgEnv hasn't existed since at least release 6.4.*, maybe earlier.
I'd suggest upgrading...
regards, tom lane
OK
So I have downloaded and set up version 6.5.3 on a test system. I then
altered the code on the example so that it no longer uses the PgEnv class.
I set my environment variable and ran the program now as below. I still get
a huge memory leak.
Can I ask you what version you are using?
callproc(){ PgDatabase* loStatsdb;
loStatsdb = new PgDatabase(""); delete loStatsdb;
};
main(){ while(1){ callproc(); };
};
Rgds
Tim Brookes
Tom Lane wrote:
> Tim Brookes <tim.brookes@mcmail.com> writes:
> > The procedure 'callproc' is called in a continuous loop to emphasise
> > this problem. Within the procedure all that happens is a new database
> > connection is made and deleted. On my system this eats memory at an
> > enormous rate.
>
> I could not reproduce such a leak with current sources.
>
> It appears that you are using a fairly old Postgres release --- class
> PgEnv hasn't existed since at least release 6.4.*, maybe earlier.
> I'd suggest upgrading...
>
> regards, tom lane
Tim Brookes <tim.brookes@mcmail.com> writes:
> So I have downloaded and set up version 6.5.3 on a test system. I then
> altered the code on the example so that it no longer uses the PgEnv class.
> I set my environment variable and ran the program now as below. I still get
> a huge memory leak.
> Can I ask you what version you are using?
Current CVS sources --- ie, 7.0RC1 plus or minus a little bit. But as
far as I can tell from the CVS logs, not much has been done to libpq++
since 6.5; there's certainly nothing in the logs about fixing memory
leaks. So I'd have expected 6.5 to behave the same. Interesting.
> callproc(){
> PgDatabase* loStatsdb;
> loStatsdb = new PgDatabase("");
> delete loStatsdb;
> };
> main(){
> while(1){
> callproc();
> };
> };
AFAIR, my test program looked just like that except it specified a
database name --- new PgDatabase("dbname=regression") or some such.
I wonder if it could be platform- or configuration-specific. What
options did you give to configure?
regards, tom lane
Tom
I did not use configure - I downloaded a set of RPMs for 6.5.3
I then just changed the program not to require PgEnv and then compiled,linked with
the libpq++ library.
Maybe I should download the sources and compile on my system, but I dont see how
that will change what I see as momory not being released in the PgDatabase
destructor.
Rgds
Tim
Tom Lane wrote:
> Tim Brookes <tim.brookes@mcmail.com> writes:
> > So I have downloaded and set up version 6.5.3 on a test system. I then
> > altered the code on the example so that it no longer uses the PgEnv class.
> > I set my environment variable and ran the program now as below. I still get
> > a huge memory leak.
>
> > Can I ask you what version you are using?
>
> Current CVS sources --- ie, 7.0RC1 plus or minus a little bit. But as
> far as I can tell from the CVS logs, not much has been done to libpq++
> since 6.5; there's certainly nothing in the logs about fixing memory
> leaks. So I'd have expected 6.5 to behave the same. Interesting.
>
> > callproc(){
> > PgDatabase* loStatsdb;
>
> > loStatsdb = new PgDatabase("");
> > delete loStatsdb;
> > };
>
> > main(){
> > while(1){
> > callproc();
> > };
> > };
>
> AFAIR, my test program looked just like that except it specified a
> database name --- new PgDatabase("dbname=regression") or some such.
>
> I wonder if it could be platform- or configuration-specific. What
> options did you give to configure?
>
> regards, tom lane
Tim Brookes wrote: > > Tom > > I did not use configure - I downloaded a set of RPMs for 6.5.3 > I then just changed the program not to require PgEnv and then compiled,linked with > the libpq++ library. To answer Tom's question: > > I wonder if it could be platform- or configuration-specific. What > > options did you give to configure? To which I can answer, if it was my RPMset he downloaded: CFLAGS="$RPM_OPT_FLAGS" ./configure --prefix=/usr \--enable-hba --enable-locale \--with-perl --with-mb=SQL_ASCII\--with-tcl--with-tk --with-x \--with-odbc --with-java \--with-python -- Lamar Owen WGCR Internet Radio 1 Peter 4:11
Ah-hah, figured it out. There *was* a recent relevant change, but the
log message for it didn't say anything about leaks. The problem is
in the "debug" code for PgConnection, which in 6.5.* looks like:
// PgConnection::connect
// establish a connection to a backend
ConnStatusType PgConnection::Connect(const char* conninfo)
{
ConnStatusType cst; // Turn the trace on
#if defined(DEBUG) FILE *debug = fopen("/tmp/trace.out","w"); PQtrace(pgConn, debug);
#endif // Connect to the database pgConn = PQconnectdb(conninfo);
This is busted in at least five ways:
1. "DEBUG" is a symbol defined in the backend header files (it's a severity level constant for elog()). So although
libpq++'sMakefile thinks it can turn the code on or off via -DDEBUG, it's mistaken; that debug-tracing code always
getscompiled. Oliver Elphick fixed that a month or so ago by changing the controlling symbol to "DEBUGFILE". So
that'swhy you see the leak in 6.5.3 and I don't see it in current sources.
2. The fopen'd file is never closed. (The destructor calls PQuntrace but that routine isn't responsible for closing
thefile.) That's the direct cause of the memory and file descriptor leak you see: a stdio file is fopen'd and never
fclose'dfor each PgConnection created.
3. The whole thing is a complete waste of time because the pgConn object doesn't exist yet --- it's not created till
thenext line! So PQtrace is being handed either a null pgConn pointer (which I believe it will silently ignore) or a
garbagepointer (which could have who-knows-what unpleasant consequences). But in no case will any tracing actually
occur. Sheesh.
4. If any tracing did occur, all of the output from (perhaps many) different PgConnection objects in different program
runswould all get dumped into the same temp file, leaving it of doubtful use to anybody.
5. One could reasonably doubt that it's a good idea to have a compiled-in option to dump the entire trace of a
program'sinteraction with the backend into a publicly readable temp file. Can you say "security hole"? Seems to me
thatthis function should require a specific request from the application, not be something that could accidentally
getinstalled as the default behavior of a system library. (Think what would happen if RPMs containing such behavior
gotdistributed...)
Perhaps something can be salvaged from the wreckage, but for now the
right answer is just to make sure that this code is not compiled.
regards, tom lane
Tom Lane wrote: > Ah-hah, figured it out. There *was* a recent relevant change, but the > log message for it didn't say anything about leaks. The problem is > in the "debug" code for PgConnection, which in 6.5.* looks like: [snip] > 1. "DEBUG" is a symbol defined in the backend header files (it's a > severity level constant for elog()). So although libpq++'s Makefile > thinks it can turn the code on or off via -DDEBUG, it's mistaken; > that debug-tracing code always gets compiled. > Oliver Elphick fixed that a month or so ago by changing the > controlling symbol to "DEBUGFILE". So that's why you see the leak > in 6.5.3 and I don't see it in current sources. > 4. If any tracing did occur, all of the output from (perhaps many) > different PgConnection objects in different program runs would all > get dumped into the same temp file, leaving it of doubtful use to > anybody. > 5. One could reasonably doubt that it's a good idea to have a compiled-in > option to dump the entire trace of a program's interaction with the > backend into a publicly readable temp file. Can you say "security > hole"? Seems to me that this function should require a specific Hmmmm.. > request from the application, not be something that could accidentally > get installed as the default behavior of a system library. (Think > what would happen if RPMs containing such behavior got distributed...) Yes, think of it. How many other such 'surprises' lurk, I wonder? > Perhaps something can be salvaged from the wreckage, but for now the > right answer is just to make sure that this code is not compiled. And to compile this code you supply _which_ configure option? Which is the default (which is how the RPMset is built)? Or is this like one of the many options in config.h.in (or config.h, depending on which side of the configure divide you prefer to edit....), like so many other options that aren't configure'able? This is as good an argument as any for a single, canonical, RPMset as I've seen in a while, Tom. I (and others in the development group who can help watchdog the RPMset) just have to be extra-diligent to make sure stuff like compiling this freakish code doesn't happen. -- Lamar Owen WGCR Internet Radio 1 Peter 4:11
Lamar Owen <lamar.owen@wgcr.org> writes:
>> Perhaps something can be salvaged from the wreckage, but for now the
>> right answer is just to make sure that this code is not compiled.
> And to compile this code you supply _which_ configure option?
It isn't a configure option --- you have to edit libpq++'s makefile
to enable it. Or, in the case of the 6.5.* code, it got turned on
anyway :-(. Fortunately there is no security hole in either 6.5.*
or current, since the code is too broken to actually produce any
trace output, compiled or not. My point was mainly that I thought
we should remove the code rather than fix it.
regards, tom lane
Tom Lane wrote: > Lamar Owen <lamar.owen@wgcr.org> writes: > >> Perhaps something can be salvaged from the wreckage, but for now the > >> right answer is just to make sure that this code is not compiled. > > And to compile this code you supply _which_ configure option? > It isn't a configure option --- you have to edit libpq++'s makefile > to enable it. Or, in the case of the 6.5.* code, it got turned on > anyway :-(. Now that is just grand. It will be a happy day once all the various configuration options, both runtime and compiletime, are brought under the GrandReUnification umbrella that Peter E is weaving right now.... > Fortunately there is no security hole in either 6.5.* > or current, since the code is too broken to actually produce any > trace output, compiled or not. That's what I thought you said. Which, in this case, a bug was a good thing -- as if the code hadn't been buggy, we'd have had a security bug. ~:-] > My point was mainly that I thought > we should remove the code rather than fix it. Who added the code in the first place? Would anyone _need_ it? Otherwise, I agree -- _can_ it, if it's not too big of a change this close to release (methinks not, but, you never know). -- Lamar Owen WGCR Internet Radio 1 Peter 4:11
Tim Brookes <tim.brookes@mcmail.com> writes:
> I have repeated my previous test, and found I am still leaking huge
> amounts of memory.
Hmph. Are you sure you are releasing query results when you're done
with them?
regards, tom lane
Tim Brookes <tim.brookes@mcmail.com> writes:
> The code is simply a constructor and destructor call of PgDatabase,
> called in succession, within an endless loop to emphasise the issue.
> But you're right, something isn't being released - within the
> destructor.
All I can say is, I see absolutely zero leakage with 7.0 code and the
attached test program. Are you sure you didn't link a 6.5 library
by mistake, or something like that?
regards, tom lane
#include <iostream.h>
#include <libpq++.h>
int main()
{ for (int i = 0; i < 10000; i++) { // Open the connection to the database and make sure it's OK PgDatabase
data("dbname=template1user=postgres"); if ( data.ConnectionBad() ) { cout << "Connection was unsuccessful..." <<
endl << "Error message returned: " << data.ErrorMessage() << endl; return 1; } } return 0;
} // End main()