From: Tom Lane <tgl@sss.pgh.pa.us>
To: Lamar Owen <lamar.owen@wgcr.org>
Cc: Tim Brookes <tim.brookes@mcmail.com>
Cc: pgsql-interfaces@postgresql.org
Subject: libpq++ tracing considered harmful (was Re: libpq++ memory problems)
Date: Thu, 20 Apr 2000 13:41:32 -0400
Message-ID: <8163.956252492@sss.pgh.pa.us> (raw)
In-Reply-To: <38FF243E.E5C0E87E@wgcr.org>
References: <38FB6FA1.C63B030B@mcmail.com>
<16483.956019092@sss.pgh.pa.us>
<38FEB8DB.DBC3460E@mcmail.com>
<2182.956243193@sss.pgh.pa.us>
<38FF2126.9525DB82@mcmail.com>
<38FF243E.E5C0E87E@wgcr.org>
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++'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.
2. The fopen'd file is never closed. (The destructor calls PQuntrace
but that routine isn't responsible for closing the file.) That's
the direct cause of the memory and file descriptor leak you see:
a stdio file is fopen'd and never fclose'd for 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 the next line!
So PQtrace is being handed either a null pgConn pointer (which I
believe it will silently ignore) or a garbage pointer (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 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
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...)
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-interfaces@postgresql.org
Cc: tgl@sss.pgh.pa.us, lamar.owen@wgcr.org, tim.brookes@mcmail.com
Subject: Re: libpq++ tracing considered harmful (was Re: libpq++ memory problems)
In-Reply-To: <8163.956252492@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox