From 0c5d5ca99c0c0c16f2e8aa2d98f256c488fada7c Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@keybit.net>
Date: Wed, 30 Jan 2013 18:52:55 +0100
Subject: [PATCH] Fix postgresql connection leaks

Return connection to pool on connection error (see #1708)
Fix leaks on persist_connection=false (#1711)

Includes testcase for #1711
---
 plugins/input/postgis/connection.hpp         |  2 +-
 plugins/input/postgis/postgis_datasource.cpp | 31 ++++++++++++--------
 tests/python_tests/postgis_test.py           | 16 ++++++++++
 3 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/plugins/input/postgis/connection.hpp b/plugins/input/postgis/connection.hpp
index 2f4bc5532..d8726aef0 100644
--- a/plugins/input/postgis/connection.hpp
+++ b/plugins/input/postgis/connection.hpp
@@ -143,7 +143,7 @@ public:
 
     bool isOK() const
     {
-        return (PQstatus(conn_) != CONNECTION_BAD);
+        return (!closed_) && (PQstatus(conn_) != CONNECTION_BAD);
     }
 
     void close()
diff --git a/plugins/input/postgis/postgis_datasource.cpp b/plugins/input/postgis/postgis_datasource.cpp
index 57b6fc907..394a036d1 100644
--- a/plugins/input/postgis/postgis_datasource.cpp
+++ b/plugins/input/postgis/postgis_datasource.cpp
@@ -113,10 +113,12 @@ postgis_datasource::postgis_datasource(parameters const& params)
     if (pool)
     {
         shared_ptr<Connection> conn = pool->borrowObject();
-        if (conn && conn->isOK())
+        if (!conn) return;
+
+        PoolGuard<shared_ptr<Connection>,
+                  shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn, pool);
+        if (conn->isOK())
         {
-            PoolGuard<shared_ptr<Connection>,
-                      shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn, pool);
 
             desc_.set_encoding(conn->client_encoding());
 
@@ -414,7 +416,7 @@ postgis_datasource::postgis_datasource(parameters const& params)
 
             rs->close();
 
-                }
+        }
     }
 }
 
@@ -429,6 +431,7 @@ postgis_datasource::~postgis_datasource()
             if (conn)
             {
                 conn->close();
+                pool->returnObject(conn);
             }
         }
     }
@@ -693,6 +696,7 @@ featureset_ptr postgis_datasource::features(const query& q) const
             if (conn)
             {
                 err_msg += conn->status();
+                pool->returnObject(conn);
             }
             else
             {
@@ -714,10 +718,11 @@ featureset_ptr postgis_datasource::features_at_point(coord2d const& pt, double t
     if (pool)
     {
         shared_ptr<Connection> conn = pool->borrowObject();
-        if (conn && conn->isOK())
-        {
-            PoolGuard<shared_ptr<Connection>, shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn, pool);
+        if (!conn) return featureset_ptr();
+        PoolGuard<shared_ptr<Connection>, shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn, pool);
 
+        if (conn->isOK())
+        {
             if (geometryColumn_.empty())
             {
                 std::ostringstream s_error;
@@ -798,10 +803,10 @@ box2d<double> postgis_datasource::envelope() const
     if (pool)
     {
         shared_ptr<Connection> conn = pool->borrowObject();
-        if (conn && conn->isOK())
+        if (!conn) return extent_;
+        PoolGuard<shared_ptr<Connection>, shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn, pool);
+        if (conn->isOK())
         {
-            PoolGuard<shared_ptr<Connection>, shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn, pool);
-
             std::ostringstream s;
 
             if (geometryColumn_.empty())
@@ -890,10 +895,10 @@ boost::optional<mapnik::datasource::geometry_t> postgis_datasource::get_geometry
     if (pool)
     {
         shared_ptr<Connection> conn = pool->borrowObject();
-        if (conn && conn->isOK())
+        if (!conn) return result;
+        PoolGuard<shared_ptr<Connection>, shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn, pool);
+        if (conn->isOK())
         {
-            PoolGuard<shared_ptr<Connection>, shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn, pool);
-
             std::ostringstream s;
             std::string g_type;
             try
diff --git a/tests/python_tests/postgis_test.py b/tests/python_tests/postgis_test.py
index 619326588..663ae71bb 100644
--- a/tests/python_tests/postgis_test.py
+++ b/tests/python_tests/postgis_test.py
@@ -495,6 +495,22 @@ if 'postgis' in mapnik.DatasourceCache.plugin_names() \
         eq_(feat['gid'],2)
         eq_(feat['int_field'],922337203685477580)
 
+    def test_persist_connection_off():
+        # NOTE: max_size should be equal or greater than
+        #       the pool size. There's currently no API to
+        #       check nor set that size, but the current 
+        #       default is 20, so we use that value. See
+        #       http://github.com/mapnik/mapnik/issues/863
+        max_size = 20
+        for i in range(0, max_size+1):
+          ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,
+                              max_size=1, # unused
+                              persist_connection=False,
+                              table='(select ST_MakePoint(0,0) as g, pg_backend_pid() as p, 1 as v) as w',
+                              geometry_field='g')
+          fs = ds.featureset()
+          eq_(fs.next()['v'], 1)
+
     atexit.register(postgis_takedown)
 
 if __name__ == "__main__":