Opened 13 years ago

Last modified 13 years ago

#1328 closed defect (fixed)

Memory leaks in OGR PostgreSQL driver

Reported by: Mateusz Łoskot Owned by: warmerdam
Priority: highest Milestone:
Component: default Version: unspecified
Severity: normal Keywords:
Cc:

Description

I detected two memory leaks occur when using PostgresQL driver:
1. In OGRPGDataSource, one of query results is not freed. It's been fixed in patch attached

2. Connection handler leaks, even if released properly. It seems there is a problem in libpq.


 Valgrind test 1
==================================================================================
$ valgrind --tool=memcheck --leak-check=full ogr/ogr2ogr -f PostgreSQL PG:'dbname=gistest user=mloskot' world_borders.shp -nln borders

==14959== ERROR SUMMARY: 6 errors from 2 contexts (suppressed: 79 from 1)
==14959== malloc/free: in use at exit: 3,446 bytes in 26 blocks.
==14959== malloc/free: 216,180 allocs, 216,154 frees, 181,534,781 bytes allocated.
==14959== For counts of detected errors, rerun with: -v
==14959== searching for pointers to 26 not-freed blocks.
==14959== checked 3,071,496 bytes.
==14959==
==14959==
==14959== 156 (36 direct, 120 indirect) bytes in 1 blocks are definitely lost in loss record 9 of 16
==14959==    at 0x401C422: malloc (vg_replace_malloc.c:149)
==14959==    by 0x5F4071B: (within /lib/tls/i686/cmov/libc-2.3.6.so)
==14959==    by 0x5F40D3E: __nss_database_lookup (in /lib/tls/i686/cmov/libc-2.3.6.so)
==14959==    by 0x4587118: ???
==14959==    by 0x5EF11B1: getpwuid_r (in /lib/tls/i686/cmov/libc-2.3.6.so)
==14959==    by 0x5B45A6E: pqGetpwuid (in /usr/lib/libpq.so.4.1)
==14959==    by 0x5B372CE: pqGetHomeDirectory (in /usr/lib/libpq.so.4.1)
==14959==    by 0x5B376BC: (within /usr/lib/libpq.so.4.1)
==14959==    by 0x5B3784E: PQconnectStart (in /usr/lib/libpq.so.4.1)
==14959==    by 0x5B37BC0: PQconnectdb (in /usr/lib/libpq.so.4.1)
==14959==    by 0x43ADB1B: OGRPGDataSource::Open(char const*, int, int) (ogrpgdatasource.cpp:291)
==14959==    by 0x43B0870: OGRPGDriver::CreateDataSource(char const*, char**) (ogrpgdriver.cpp:106)
==14959==
==14959==
==14959== 2,672 (112 direct, 2,560 indirect) bytes in 1 blocks are definitely lost in loss record 14 of 16
==14959==    at 0x401C422: malloc (vg_replace_malloc.c:149)
==14959==    by 0x5B37DBD: PQmakeEmptyPGresult (in /usr/lib/libpq.so.4.1)
==14959==    by 0x5B40B11: pqParseInput3 (in /usr/lib/libpq.so.4.1)
==14959==    by 0x5B3861A: (within /usr/lib/libpq.so.4.1)
==14959==    by 0x5B38FCB: PQgetResult (in /usr/lib/libpq.so.4.1)
==14959==    by 0x5B390D0: (within /usr/lib/libpq.so.4.1)
==14959==    by 0x43B03B9: OGRPGDataSource::CreateLayer(char const*, OGRSpatialReference*, OGRwkbGeometryType, char**) (ogrpgdatasource.cpp:858)
==14959==    by 0x8049809: TranslateLayer(OGRDataSource*, OGRLayer*, OGRDataSource*, char**, char const*, int, OGRSpatialReference*, OGRSpatialReference*, char**, int, int, int) (ogr2ogr.cpp:771)
==14959==    by 0x804B1F2: main (ogr2ogr.cpp:548)
==14959==
==14959== LEAK SUMMARY:
==14959==    definitely lost: 148 bytes in 2 blocks.
==14959==    indirectly lost: 2,680 bytes in 12 blocks.
==14959==      possibly lost: 0 bytes in 0 blocks.
==14959==    still reachable: 618 bytes in 12 blocks.
==14959==         suppressed: 0 bytes in 0 blocks.
==================================================================================

The SECOND error reported above is related to 1st leak described at the 
beginning of my report.
After fixed it, valgrind reports looks like this

 Valgrind test 2
==================================================================================
$ valgrind --tool=memcheck --leak-check=full ogr/ogr2ogr -f PostgreSQL PG:'dbname=gistest user=mloskot' world_borders.shp -nln borders

==16890== ERROR SUMMARY: 6 errors from 2 contexts (suppressed: 79 from 1)
==16890== malloc/free: in use at exit: 774 bytes in 23 blocks.
==16890== malloc/free: 216,181 allocs, 216,158 frees, 181,559,781 bytes allocated.
==16890== For counts of detected errors, rerun with: -v
==16890== searching for pointers to 23 not-freed blocks.
==16890== checked 3,071,496 bytes.
==16890==
==16890==
==16890== 156 (36 direct, 120 indirect) bytes in 1 blocks are definitely lost in loss record 9 of 13
==16890==    at 0x401C422: malloc (vg_replace_malloc.c:149)
==16890==    by 0x5F4071B: (within /lib/tls/i686/cmov/libc-2.3.6.so)
==16890==    by 0x5F40D3E: __nss_database_lookup (in /lib/tls/i686/cmov/libc-2.3.6.so)
==16890==    by 0x4587118: ???
==16890==    by 0x5EF11B1: getpwuid_r (in /lib/tls/i686/cmov/libc-2.3.6.so)
==16890==    by 0x5B45A6E: pqGetpwuid (in /usr/lib/libpq.so.4.1)
==16890==    by 0x5B372CE: pqGetHomeDirectory (in /usr/lib/libpq.so.4.1)
==16890==    by 0x5B376BC: (within /usr/lib/libpq.so.4.1)
==16890==    by 0x5B3784E: PQconnectStart (in /usr/lib/libpq.so.4.1)
==16890==    by 0x5B37BC0: PQconnectdb (in /usr/lib/libpq.so.4.1)
==16890==    by 0x43ADB1B: OGRPGDataSource::Open(char const*, int, int) (ogrpgdatasource.cpp:296)
==16890==    by 0x43B08C4: OGRPGDriver::CreateDataSource(char const*, char**) (ogrpgdriver.cpp:106)
==16890==
==16890== LEAK SUMMARY:
==16890==    definitely lost: 36 bytes in 1 blocks.
==16890==    indirectly lost: 120 bytes in 10 blocks.
==16890==      possibly lost: 0 bytes in 0 blocks.
==16890==    still reachable: 618 bytes in 12 blocks.
==16890==         suppressed: 0 bytes in 0 blocks.
==================================================================================

As you can see, only connection resource is reported as a leak, somewhere in the libpq. I've tracked that connection established in line grpgdatasource.cpp:296 is finished in OGRPGDataSource::~OGRPGDataSource(). It seems the libpq delays the connection handle release in time.
AFAIK, it's common situation for result handler, as described in book "Beginning Databases with PostgreSQL", 2nd Edition:

"Note that results are not cleared automatically, even when the connection is closed, so they can be kept indefinitely if required"


To confirm it's not any leak anywhere in the OGR, here is a very simple test program that connects and disconnects with PostgreSQL 'test' database. As you 
can see in the report below, valgrind still reports the same leak as in reports above.

///////////////////////////////////////////////////////////////////////////////
// pqconn_test.cpp
// Compile:
// g++ -g -Wall -pedantic -I/usr/include/postgresql pqconn.cpp -o pqconn -L/usr/lib -lpq

#include <libpq-fe.h>
#include <cassert>
#include <iostream>
using namespace std;

int main()
{
    PGconn* conn = 0;

    // Connect
    conn = PQconnectdb("dbname=test");
    assert(0 != conn);
    assert(CONNECTION_OK == PQstatus(conn));

    cout << "Connected to: " << PQdb(conn) << endl;

    // Disconnect
    PQfinish(conn);
    conn = 0;
    cout << "Disconnected." << endl;
    
    return 0;
}
///////////////////////////////////////////////////////////////////////////////

Here is valgind report for this program:

==================================================================================
mloskot:~/workshop/pgsql$ valgrind --tool=memcheck --leak-check=yes ./pqconn

==17238== ERROR SUMMARY: 6 errors from 2 contexts (suppressed: 43 from 1)
==17238== malloc/free: in use at exit: 156 bytes in 11 blocks.
==17238== malloc/free: 139 allocs, 128 frees, 44,263 bytes allocated.
==17238== For counts of detected errors, rerun with: -v
==17238== searching for pointers to 11 not-freed blocks.
==17238== checked 494,052 bytes.
==17238==
==17238==
==17238== 156 (36 direct, 120 indirect) bytes in 1 blocks are definitely lost in loss record 1 of 3
==17238==    at 0x401C422: malloc (vg_replace_malloc.c:149)
==17238==    by 0x424771B: (within /lib/tls/i686/cmov/libc-2.3.6.so)
==17238==    by 0x4247D3E: __nss_database_lookup (in /lib/tls/i686/cmov/libc-2.3.6.so)
==17238==    by 0x4631118: ???
==17238==    by 0x41F81B1: getpwuid_r (in /lib/tls/i686/cmov/libc-2.3.6.so)
==17238==    by 0x4054A6E: pqGetpwuid (in /usr/lib/libpq.so.4.1)
==17238==    by 0x4043548: pg_fe_getauthname (in /usr/lib/libpq.so.4.1)
==17238==    by 0x4045AA9: (within /usr/lib/libpq.so.4.1)
==17238==    by 0x4045BE1: (within /usr/lib/libpq.so.4.1)
==17238==    by 0x404683B: PQconnectStart (in /usr/lib/libpq.so.4.1)
==17238==    by 0x4046BC0: PQconnectdb (in /usr/lib/libpq.so.4.1)
==17238==    by 0x8048833: main (pqconn.cpp:12)
==17238==
==17238== LEAK SUMMARY:
==17238==    definitely lost: 36 bytes in 1 blocks.
==17238==    indirectly lost: 120 bytes in 10 blocks.
==17238==      possibly lost: 0 bytes in 0 blocks.
==17238==    still reachable: 0 bytes in 0 blocks.
==17238==         suppressed: 0 bytes in 0 blocks.
==================================================================================


Summarizing, if my understanding of the first leak about not released query result is correct, I'd suggest to apply the submitted patch.
The second leak issue is worth to memorize for future.

Attachments (1)

ogr-pg-memleak-mloskot-20061014.patch (961 bytes) - added by Mateusz Łoskot 13 years ago.
Memory leak fix in ogrpgdatasource.cpp

Download all attachments as: .zip

Change History (3)

Changed 13 years ago by Mateusz Łoskot

Memory leak fix in ogrpgdatasource.cpp

comment:1 Changed 13 years ago by Mateusz Łoskot

Taking this bug.

comment:2 Changed 13 years ago by Mateusz Łoskot

This bug has been fixed.
Note: See TracTickets for help on using tickets.