Skip to content

Commit

Permalink
Merge pull request #1823 from thjc/master
Browse files Browse the repository at this point in the history 
Fix for #1588 Postgis: Concurrency problem with CursorResultSet. Thanks @thjc
  • Loading branch information
@rcoup
rcoup committed May 6, 2013
2 parents faa8394 + ac09541 commit 4f40993
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 71 deletions.
80 changes: 19 additions & 61 deletions include/mapnik/pool.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,28 +42,6 @@

namespace mapnik
{
template <typename T, typename PoolT>
class PoolGuard
{
private:
const T& obj_;
PoolT& pool_;
public:
explicit PoolGuard(const T& ptr,PoolT& pool)
: obj_(ptr),
pool_(pool) {}

~PoolGuard()
{
pool_->returnObject(obj_);
}

private:
PoolGuard();
PoolGuard(const PoolGuard&);
PoolGuard& operator=(const PoolGuard&);
};

template <typename T,template <typename> class Creator>
class Pool : private mapnik::noncopyable
{
Expand All @@ -73,8 +51,7 @@ class Pool : private mapnik::noncopyable
Creator<T> creator_;
unsigned initialSize_;
unsigned maxSize_;
ContType usedPool_;
ContType unusedPool_;
ContType pool_;
#ifdef MAPNIK_THREADSAFE
mutable boost::mutex mutex_;
#endif
Expand All @@ -89,7 +66,7 @@ class Pool : private mapnik::noncopyable
{
HolderType conn(creator_());
if (conn->isOK())
unusedPool_.push_back(conn);
pool_.push_back(conn);
}
}

Expand All @@ -98,31 +75,33 @@ class Pool : private mapnik::noncopyable
#ifdef MAPNIK_THREADSAFE
mutex::scoped_lock lock(mutex_);
#endif
typename ContType::iterator itr=unusedPool_.begin();
while ( itr!=unusedPool_.end())
{
MAPNIK_LOG_DEBUG(pool) << "pool: Borrow instance=" << (*itr).get();

if ((*itr)->isOK())
typename ContType::iterator itr=pool_.begin();
while ( itr!=pool_.end())
{
if (!itr->unique())
{
++itr;
}
else if ((*itr)->isOK())
{
usedPool_.push_back(*itr);
unusedPool_.erase(itr);
return usedPool_.back();
MAPNIK_LOG_DEBUG(pool) << "pool: Borrow instance=" << (*itr).get();
return *itr;
}
else
{
MAPNIK_LOG_DEBUG(pool) << "pool: Bad connection (erase) instance=" << (*itr).get();

itr=unusedPool_.erase(itr);
itr=pool_.erase(itr);
}
}
// all connection have been taken, check if we allowed to grow pool
if (usedPool_.size() < maxSize_)
if (pool_.size() < maxSize_)
{
HolderType conn(creator_());
if (conn->isOK())
{
usedPool_.push_back(conn);
pool_.push_back(conn);

MAPNIK_LOG_DEBUG(pool) << "pool: Create connection=" << conn.get();

Expand All @@ -132,33 +111,12 @@ class Pool : private mapnik::noncopyable
return HolderType();
}

void returnObject(HolderType obj)
{
#ifdef MAPNIK_THREADSAFE
mutex::scoped_lock lock(mutex_);
#endif
typename ContType::iterator itr=usedPool_.begin();
while (itr != usedPool_.end())
{
if (obj.get()==(*itr).get())
{
MAPNIK_LOG_DEBUG(pool) << "pool: Return instance=" << (*itr).get();

unusedPool_.push_back(*itr);
usedPool_.erase(itr);
return;
}
++itr;
}
}

std::pair<unsigned,unsigned> size() const
unsigned size() const
{
#ifdef MAPNIK_THREADSAFE
mutex::scoped_lock lock(mutex_);
#endif
std::pair<unsigned,unsigned> size(unusedPool_.size(),usedPool_.size());
return size;
return pool_.size();
}

unsigned max_size() const
Expand Down Expand Up @@ -193,7 +151,7 @@ class Pool : private mapnik::noncopyable
if (size > initialSize_)
{
initialSize_ = size;
unsigned total_size = usedPool_.size() + unusedPool_.size();
unsigned total_size = pool_.size();
// ensure we don't have ghost obj's in the pool.
if (total_size < initialSize_)
{
Expand All @@ -203,7 +161,7 @@ class Pool : private mapnik::noncopyable
{
HolderType conn(creator_());
if (conn->isOK())
unusedPool_.push_back(conn);
pool_.push_back(conn);
}
}
}
Expand Down
10 changes: 0 additions & 10 deletions plugins/input/postgis/postgis_datasource.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ const std::string postgis_datasource::GEOMETRY_COLUMNS = "geometry_columns";
const std::string postgis_datasource::SPATIAL_REF_SYS = "spatial_ref_system";

using boost::shared_ptr;
using mapnik::PoolGuard;
using mapnik::attribute_descriptor;

postgis_datasource::postgis_datasource(parameters const& params)
Expand Down Expand Up @@ -115,8 +114,6 @@ postgis_datasource::postgis_datasource(parameters const& params)
shared_ptr<Connection> conn = pool->borrowObject();
if (!conn) return;

PoolGuard<shared_ptr<Connection>,
shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn, pool);
if (conn->isOK())
{

Expand Down Expand Up @@ -431,7 +428,6 @@ postgis_datasource::~postgis_datasource()
if (conn)
{
conn->close();
pool->returnObject(conn);
}
}
}
Expand Down Expand Up @@ -603,8 +599,6 @@ featureset_ptr postgis_datasource::features(const query& q) const
shared_ptr<Connection> conn = pool->borrowObject();
if (conn && conn->isOK())
{
PoolGuard<shared_ptr<Connection>, shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn ,pool);

if (geometryColumn_.empty())
{
std::ostringstream s_error;
Expand Down Expand Up @@ -696,7 +690,6 @@ featureset_ptr postgis_datasource::features(const query& q) const
if (conn)
{
err_msg += conn->status();
pool->returnObject(conn);
}
else
{
Expand All @@ -719,7 +712,6 @@ featureset_ptr postgis_datasource::features_at_point(coord2d const& pt, double t
{
shared_ptr<Connection> conn = pool->borrowObject();
if (!conn) return featureset_ptr();
PoolGuard<shared_ptr<Connection>, shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn, pool);

if (conn->isOK())
{
Expand Down Expand Up @@ -804,7 +796,6 @@ box2d<double> postgis_datasource::envelope() const
{
shared_ptr<Connection> conn = pool->borrowObject();
if (!conn) return extent_;
PoolGuard<shared_ptr<Connection>, shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn, pool);
if (conn->isOK())
{
std::ostringstream s;
Expand Down Expand Up @@ -896,7 +887,6 @@ boost::optional<mapnik::datasource::geometry_t> postgis_datasource::get_geometry
{
shared_ptr<Connection> conn = pool->borrowObject();
if (!conn) return result;
PoolGuard<shared_ptr<Connection>, shared_ptr< Pool<Connection,ConnectionCreator> > > guard(conn, pool);
if (conn->isOK())
{
std::ostringstream s;
Expand Down

0 comments on commit 4f40993

Please sign in to comment.