Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix postgresql connection leaks on error #1709

Merged
merged 1 commit into from

2 participants

@strk

See #1708

@springmeyer
Owner

Before considering backporting I think we need tests that are able to replicate the problem before the patch and clearly show it is fixed after.

@strk

It takes a memory checker to test this. Do we want to start a valgrind-based suite ?

@strk

The equivalent of this patch was merged in master (#1764)

@springmeyer springmeyer closed this
@springmeyer springmeyer reopened this
@springmeyer springmeyer merged commit 88f8d99 into mapnik:2.1.x
@springmeyer
Owner

note: amended in c6fd387 to avoid crash in constructor if the connection is not initialized. I did not replicate this but it looks like an obvious problem that could occur given the way the patch was made.

@strk strk deleted the strk:2.1.x-out_of_pool_slots branch
@strk

Yep, thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 31, 2013
  1. @strk
This page is out of date. Refresh to see the latest.
View
3  plugins/input/postgis/connection.hpp
@@ -71,6 +71,7 @@ class Connection
}
s << "\n" << connection_str;
+ PQfinish(conn_);
throw mapnik::datasource_exception(s.str());
}
}
@@ -152,7 +153,7 @@ class Connection
bool isOK() const
{
- return (PQstatus(conn_) != CONNECTION_BAD);
+ return (!closed_) && (PQstatus(conn_) != CONNECTION_BAD);
}
void close()
View
41 plugins/input/postgis/postgis_datasource.cpp
@@ -127,11 +127,12 @@ void postgis_datasource::bind() const
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;
+ PoolGuard<shared_ptr<Connection>,
+ shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn, pool);
+ if (conn->isOK())
+ {
desc_.set_encoding(conn->client_encoding());
if (geometry_table_.empty())
@@ -435,6 +436,7 @@ postgis_datasource::~postgis_datasource()
if (conn)
{
conn->close();
+ pool->returnObject(conn);
}
}
}
@@ -706,7 +708,17 @@ featureset_ptr postgis_datasource::features(const query& q) const
}
else
{
- throw mapnik::datasource_exception("Postgis Plugin: bad connection");
+ std::string err_msg = "Postgis Plugin:";
+ if (conn)
+ {
+ err_msg += " Bad connection";
+ pool->returnObject(conn);
+ }
+ else
+ {
+ err_msg += " Null connection";
+ }
+ throw mapnik::datasource_exception(err_msg);
}
}
@@ -729,10 +741,11 @@ featureset_ptr postgis_datasource::features_at_point(coord2d const& pt) const
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;
@@ -819,10 +832,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;
boost::optional<mapnik::boolean> estimate_extent =
@@ -920,10 +933,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;
Something went wrong with that request. Please try again.