Use shared pointer reference count to track pools

By using the shared pointer reference count to track the connections
in the connection pool we no longer have to explicitly return them
to the pool.

This resolves an issue where cursor feature sets were returning their
connections before they were done with them, causing a potential race
with another connection user.
This commit is contained in:
Toby Collett 2013-04-29 05:43:39 +00:00
parent faa8394ef6
commit ac09541243
2 changed files with 19 additions and 71 deletions

View file

@ -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
{
@ -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
@ -89,7 +66,7 @@ public:
{
HolderType conn(creator_());
if (conn->isOK())
unusedPool_.push_back(conn);
pool_.push_back(conn);
}
}
@ -98,31 +75,33 @@ public:
#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())
{
usedPool_.push_back(*itr);
unusedPool_.erase(itr);
return usedPool_.back();
++itr;
}
else if ((*itr)->isOK())
{
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();
@ -132,33 +111,12 @@ public:
return HolderType();
}
void returnObject(HolderType obj)
unsigned size() const
{
#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
{
#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
@ -193,7 +151,7 @@ public:
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_)
{
@ -203,7 +161,7 @@ public:
{
HolderType conn(creator_());
if (conn->isOK())
unusedPool_.push_back(conn);
pool_.push_back(conn);
}
}
}

View file

@ -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)
@ -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())
{
@ -431,7 +428,6 @@ postgis_datasource::~postgis_datasource()
if (conn)
{
conn->close();
pool->returnObject(conn);
}
}
}
@ -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;
@ -696,7 +690,6 @@ featureset_ptr postgis_datasource::features(const query& q) const
if (conn)
{
err_msg += conn->status();
pool->returnObject(conn);
}
else
{
@ -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())
{
@ -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;
@ -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;