Opened 12 years ago
Closed 12 years ago
#4533 closed task (fixed)
PostGIS Raster driver: Cache connections
Reported by: | dzwarg | Owned by: | dzwarg |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | GDAL_Raster | Version: | svn-trunk |
Severity: | normal | Keywords: | postgis_raster |
Cc: | jorgearevalo |
Description
Cache connections to the postgres database in the PostGISRasterDriver.
Also, use PG* environment variables as fallback when settings are not provided in the connection string.
Attachments (2)
Change History (12)
by , 12 years ago
Attachment: | conninfo.patch added |
---|
by , 12 years ago
Attachment: | conninfo.2.patch added |
---|
This patch has been run through valgrind -- no memleaks detected. Rock'n'roll with connection caching.
comment:1 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Patch applied in r24088. I close the ticket.
comment:2 by , 12 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
This crashes badly when the connection cannot be opened (I get a segfault when running wktraster.py with no postgis raster enabled database).
Here's the Valgrind trace :
$ valgrind gdalinfo "PG:host='localhost' dbname='gisdb' user='gis' password='gis' schema='gis_schema' " ==15194== Memcheck, a memory error detector ==15194== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al. ==15194== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info ==15194== Command: gdalinfo PG:host='localhost'\ dbname='gisdb'\ user='gis'\ password='gis'\ schema='gis_schema'\ ==15194== ERROR 1: Port parameter must be provided, or PGPORT environment variable must be set. Please set the port and try again. ==15194== Invalid read of size 1 ==15194== at 0x4C2A5AB: bcmp (mc_replace_strmem.c:889) ==15194== by 0x580EF0A: VSICheckMarker(char*) (cpl_vsisimple.cpp:472) ==15194== by 0x580F196: VSIFree (cpl_vsisimple.cpp:552) ==15194== by 0x5648BBC: GetConnection(char const*, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1525) ==15194== by 0x5648CF3: PostGISRasterDataset::Open(GDALOpenInfo*) (postgisrasterdataset.cpp:1579) ==15194== by 0x577DF51: GDALOpenInternal(GDALOpenInfo&, char const* const*) (gdaldataset.cpp:2236) ==15194== by 0x577DE04: GDALOpenInternal(char const*, GDALAccess, char const* const*) (gdaldataset.cpp:2208) ==15194== by 0x577DDBE: GDALOpen (gdaldataset.cpp:2199) ==15194== by 0x40328C: main (gdalinfo.c:173) ==15194== Address 0x16608380 is 0 bytes inside a block of size 26 free'd ==15194== at 0x4C2779F: free (vg_replace_malloc.c:427) ==15194== by 0x580F246: VSIFree (cpl_vsisimple.cpp:572) ==15194== by 0x56487B3: GetConnectionInfo(char const*, char**, char**, char**, char**, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1423) ==15194== by 0x5648AB9: GetConnection(char const*, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1498) ==15194== by 0x5648CF3: PostGISRasterDataset::Open(GDALOpenInfo*) (postgisrasterdataset.cpp:1579) ==15194== by 0x577DF51: GDALOpenInternal(GDALOpenInfo&, char const* const*) (gdaldataset.cpp:2236) ==15194== by 0x577DE04: GDALOpenInternal(char const*, GDALAccess, char const* const*) (gdaldataset.cpp:2208) ==15194== by 0x577DDBE: GDALOpen (gdaldataset.cpp:2199) ==15194== by 0x40328C: main (gdalinfo.c:173) ==15194== ==15194== Invalid read of size 1 ==15194== at 0x580EF17: VSICheckMarker(char*) (cpl_vsisimple.cpp:476) ==15194== by 0x580F196: VSIFree (cpl_vsisimple.cpp:552) ==15194== by 0x5648BBC: GetConnection(char const*, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1525) ==15194== by 0x5648CF3: PostGISRasterDataset::Open(GDALOpenInfo*) (postgisrasterdataset.cpp:1579) ==15194== by 0x577DF51: GDALOpenInternal(GDALOpenInfo&, char const* const*) (gdaldataset.cpp:2236) ==15194== by 0x577DE04: GDALOpenInternal(char const*, GDALAccess, char const* const*) (gdaldataset.cpp:2208) ==15194== by 0x577DDBE: GDALOpen (gdaldataset.cpp:2199) ==15194== by 0x40328C: main (gdalinfo.c:173) ==15194== Address 0x16608383 is 3 bytes inside a block of size 26 free'd ==15194== at 0x4C2779F: free (vg_replace_malloc.c:427) ==15194== by 0x580F246: VSIFree (cpl_vsisimple.cpp:572) ==15194== by 0x56487B3: GetConnectionInfo(char const*, char**, char**, char**, char**, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1423) ==15194== by 0x5648AB9: GetConnection(char const*, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1498) ==15194== by 0x5648CF3: PostGISRasterDataset::Open(GDALOpenInfo*) (postgisrasterdataset.cpp:1579) ==15194== by 0x577DF51: GDALOpenInternal(GDALOpenInfo&, char const* const*) (gdaldataset.cpp:2236) ==15194== by 0x577DE04: GDALOpenInternal(char const*, GDALAccess, char const* const*) (gdaldataset.cpp:2208) ==15194== by 0x577DDBE: GDALOpen (gdaldataset.cpp:2199) ==15194== by 0x40328C: main (gdalinfo.c:173) ==15194== ==15194== Invalid read of size 1 ==15194== at 0x580EF25: VSICheckMarker(char*) (cpl_vsisimple.cpp:476) ==15194== by 0x580F196: VSIFree (cpl_vsisimple.cpp:552) ==15194== by 0x5648BBC: GetConnection(char const*, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1525) ==15194== by 0x5648CF3: PostGISRasterDataset::Open(GDALOpenInfo*) (postgisrasterdataset.cpp:1579) ==15194== by 0x577DF51: GDALOpenInternal(GDALOpenInfo&, char const* const*) (gdaldataset.cpp:2236) ==15194== by 0x577DE04: GDALOpenInternal(char const*, GDALAccess, char const* const*) (gdaldataset.cpp:2208) ==15194== by 0x577DDBE: GDALOpen (gdaldataset.cpp:2199) ==15194== by 0x40328C: main (gdalinfo.c:173) ==15194== Address 0x16608382 is 2 bytes inside a block of size 26 free'd ==15194== at 0x4C2779F: free (vg_replace_malloc.c:427) ==15194== by 0x580F246: VSIFree (cpl_vsisimple.cpp:572) ==15194== by 0x56487B3: GetConnectionInfo(char const*, char**, char**, char**, char**, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1423) ==15194== by 0x5648AB9: GetConnection(char const*, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1498) ==15194== by 0x5648CF3: PostGISRasterDataset::Open(GDALOpenInfo*) (postgisrasterdataset.cpp:1579) ==15194== by 0x577DF51: GDALOpenInternal(GDALOpenInfo&, char const* const*) (gdaldataset.cpp:2236) ==15194== by 0x577DE04: GDALOpenInternal(char const*, GDALAccess, char const* const*) (gdaldataset.cpp:2208) ==15194== by 0x577DDBE: GDALOpen (gdaldataset.cpp:2199) ==15194== by 0x40328C: main (gdalinfo.c:173) ==15194== ==15194== Invalid read of size 1 ==15194== at 0x580EF33: VSICheckMarker(char*) (cpl_vsisimple.cpp:476) ==15194== by 0x580F196: VSIFree (cpl_vsisimple.cpp:552) ==15194== by 0x5648BBC: GetConnection(char const*, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1525) ==15194== by 0x5648CF3: PostGISRasterDataset::Open(GDALOpenInfo*) (postgisrasterdataset.cpp:1579) ==15194== by 0x577DF51: GDALOpenInternal(GDALOpenInfo&, char const* const*) (gdaldataset.cpp:2236) ==15194== by 0x577DE04: GDALOpenInternal(char const*, GDALAccess, char const* const*) (gdaldataset.cpp:2208) ==15194== by 0x577DDBE: GDALOpen (gdaldataset.cpp:2199) ==15194== by 0x40328C: main (gdalinfo.c:173) ==15194== Address 0x16608381 is 1 bytes inside a block of size 26 free'd ==15194== at 0x4C2779F: free (vg_replace_malloc.c:427) ==15194== by 0x580F246: VSIFree (cpl_vsisimple.cpp:572) ==15194== by 0x56487B3: GetConnectionInfo(char const*, char**, char**, char**, char**, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1423) ==15194== by 0x5648AB9: GetConnection(char const*, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1498) ==15194== by 0x5648CF3: PostGISRasterDataset::Open(GDALOpenInfo*) (postgisrasterdataset.cpp:1579) ==15194== by 0x577DF51: GDALOpenInternal(GDALOpenInfo&, char const* const*) (gdaldataset.cpp:2236) ==15194== by 0x577DE04: GDALOpenInternal(char const*, GDALAccess, char const* const*) (gdaldataset.cpp:2208) ==15194== by 0x577DDBE: GDALOpen (gdaldataset.cpp:2199) ==15194== by 0x40328C: main (gdalinfo.c:173) ==15194== ==15194== Invalid read of size 1 ==15194== at 0x580EF3D: VSICheckMarker(char*) (cpl_vsisimple.cpp:476) ==15194== by 0x580F196: VSIFree (cpl_vsisimple.cpp:552) ==15194== by 0x5648BBC: GetConnection(char const*, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1525) ==15194== by 0x5648CF3: PostGISRasterDataset::Open(GDALOpenInfo*) (postgisrasterdataset.cpp:1579) ==15194== by 0x577DF51: GDALOpenInternal(GDALOpenInfo&, char const* const*) (gdaldataset.cpp:2236) ==15194== by 0x577DE04: GDALOpenInternal(char const*, GDALAccess, char const* const*) (gdaldataset.cpp:2208) ==15194== by 0x577DDBE: GDALOpen (gdaldataset.cpp:2199) ==15194== by 0x40328C: main (gdalinfo.c:173) ==15194== Address 0x16608380 is 0 bytes inside a block of size 26 free'd ==15194== at 0x4C2779F: free (vg_replace_malloc.c:427) ==15194== by 0x580F246: VSIFree (cpl_vsisimple.cpp:572) ==15194== by 0x56487B3: GetConnectionInfo(char const*, char**, char**, char**, char**, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1423) ==15194== by 0x5648AB9: GetConnection(char const*, char**, char**, char**, char**, char**, int*, int*) (postgisrasterdataset.cpp:1498) ==15194== by 0x5648CF3: PostGISRasterDataset::Open(GDALOpenInfo*) (postgisrasterdataset.cpp:1579) ==15194== by 0x577DF51: GDALOpenInternal(GDALOpenInfo&, char const* const*) (gdaldataset.cpp:2236) ==15194== by 0x577DE04: GDALOpenInternal(char const*, GDALAccess, char const* const*) (gdaldataset.cpp:2208) ==15194== by 0x577DDBE: GDALOpen (gdaldataset.cpp:2199) ==15194== by 0x40328C: main (gdalinfo.c:173) ==15194== ERROR 1: Inconsistant use of VSI memory allocation primitives for 0x16608380 : MISV ==15194== ==15194== HEAP SUMMARY: ==15194== in use at exit: 249,153 bytes in 2,578 blocks ==15194== total heap usage: 2,823 allocs, 245 frees, 353,484 bytes allocated ==15194== ==15194== LEAK SUMMARY: ==15194== definitely lost: 1,142 bytes in 67 blocks ==15194== indirectly lost: 0 bytes in 0 blocks ==15194== possibly lost: 153,991 bytes in 1,967 blocks ==15194== still reachable: 94,020 bytes in 544 blocks ==15194== suppressed: 0 bytes in 0 blocks ==15194== Rerun with --leak-check=full to see details of leaked memory ==15194== ==15194== For counts of detected and suppressed errors, rerun with: -v ==15194== ERROR SUMMARY: 5 errors from 5 contexts (suppressed: 16 from 6) Abandon
It would be good to add in the wkraster.py file a test with an invalid connection string by the way.
comment:3 by , 12 years ago
Status: | reopened → new |
---|
comment:4 by , 12 years ago
Cc: | added |
---|
comment:5 by , 12 years ago
Status: | new → assigned |
---|
comment:6 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | assigned → new |
comment:7 by , 12 years ago
rouault: I cannot reproduce this bug. I tried opening a connection to a garbage database:
dzwarg@raster$ gdalinfo "PG:host='localhost' dbname='junk' user='postgres' port='5432'" ERROR 1: PGconnectcb failed: FATAL: database "junk" does not exist ERROR 1: Couldn't establish a database connection gdalinfo failed - unable to open 'PG:host='localhost' dbname='junk' user='postgres' port='5432''. dzwarg@raster$
Then I tried opening a connection to a non-spatial enabled database:
dzwarg@raster$ gdalinfo "PG:host='localhost' dbname='postgres' user='postgres' port='5432'" ERROR 1: Error checking geometry type existence. Is PostGIS correctly installed?: gdalinfo failed - unable to open 'PG:host='localhost' dbname='postgres' user='postgres' port='5432''. dzwarg@raster$
Then I tried opening a connection to a non-spatial enabled database, and left out the PGPORT:
dzwarg@raster$ gdalinfo "PG:host='localhost' dbname='postgres' user='postgres'" ERROR 1: Port parameter must be provided, or PGPORT environment variable must be set. Please set the port and try again. gdalinfo failed - unable to open 'PG:host='localhost' dbname='postgres' user='postgres''. dzwarg@raster$
And I can't get it to segfault. Can you provide any more information about the crash?
comment:8 by , 12 years ago
I hoped it would have been pretty obvious, given the above pasted Valgrind trace ;-)
It is possible than you don't run into a segfault systematically if you are lucky, but I'd encourage you to edit your copy of port/cpl_vsisimple.cpp and uncomment the #define DEBUG_VSIMALLOC line (please don't commit the modified file however, this is just for debugging purposes). Or you edit your GDALmake.opt and add -DDDEBUG_VSIMALLOC to CFLAGS and CXXFLAGS. It includes a detection for double frees when using the VSI/CPL allocation routines. But even without that, Valgrind would catch it too.
Well, here's the fix (please test it works with working connection strings) :
r24096 /trunk/gdal/frmts/postgisraster/postgisrasterdataset.cpp: Postgisraster: fix double free (and make the caller responsible for cleaning up return arguments)
By the way, I don't understand why the connection string is parsed such that a host, user and password must be provided. Only the dbname should be compulsory, like for the OGR PG driver. I can do "ogrinfo pg:dbname=autotest" and this connects through a unix socket with my unix user".
comment:9 by , 12 years ago
Keywords: | postgis_raster added; postgisraster removed |
---|
comment:10 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
This fix works with working connection strings. Thank you.
Pool connections in the driver. Get all connections from the driver. This patch includes a fix for the bug reported to PostGIS Raster #1422. This patch also includes the patch for #4530.