Author: wyoung
Date: Thu Dec 27 04:18:04 2007
New Revision: 2028
URL: http://svn.gna.org/viewcvs/mysqlpp?rev=2028&view=rev
Log:
Fixes to thread safety, memory management and last-used time handling in
ConnectionPool. Initial patch by Jonathan Wakely, greatly reworked.
Modified:
trunk/lib/cpool.cpp
trunk/lib/cpool.h
Modified: trunk/lib/cpool.cpp
URL:
http://svn.gna.org/viewcvs/mysqlpp/trunk/lib/cpool.cpp?rev=2028&r1=2027&r2=2028&view=diff
==============================================================================
--- trunk/lib/cpool.cpp (original)
+++ trunk/lib/cpool.cpp Thu Dec 27 04:18:04 2007
@@ -1,9 +1,10 @@
/***********************************************************************
cpool.cpp - Implements the ConnectionPool class.
- Copyright (c) 2007 by Educational Technology Resources, Inc.
- Others may also hold copyrights on code in this file. See the
- CREDITS file in the top directory of the distribution for details.
+ Copyright (c) 2007 by Educational Technology Resources, Inc. and
+ (c) 2007 by Jonathan Wakely. Others may also hold copyrights on
+ code in this file. See the CREDITS file in the top directory of
+ the distribution for details.
This file is part of MySQL++.
@@ -27,7 +28,71 @@
#include "connection.h"
+#include <algorithm>
+#include <functional>
+
namespace mysqlpp {
+
+
+/// \brief Functor to test whether a given ConnectionInfo object is
+/// "too old".
+///
+/// This is a template only because ConnectionInfo is private. Making
+/// it a template means the private type is only used at the point of
+/// instantiation, where it is accessible.
+
+template <typename ConnInfoT>
+class TooOld : std::unary_function<ConnInfoT, bool>
+{
+public:
+ TooOld(unsigned int tmax) :
+ min_age_(time(0) - tmax)
+ {
+ }
+
+ bool operator()(const ConnInfoT& conn_info) const
+ {
+ return !conn_info.in_use && conn_info.last_used <= min_age_;
+ }
+
+private:
+ time_t min_age_;
+};
+
+
+
+//// clear /////////////////////////////////////////////////////////////
+// Destroy all connections in the pool and drain pool.
+
+void
+ConnectionPool::clear()
+{
+ ScopedLock lock(mutex_); // ensure we're not interfered with
+
+ for (PoolIt it = pool_.begin(); it != pool_.end(); ++it) {
+ destroy(it->conn);
+ }
+ pool_.clear();
+}
+
+
+//// find_mru //////////////////////////////////////////////////////////
+// Find most recently used available connection. Uses operator< for
+// ConnectionInfo to order pool with MRU connection last. Returns 0 if
+// there are no connections not in use.
+
+Connection*
+ConnectionPool::find_mru()
+{
+ PoolIt mru = std::max_element(pool_.begin(), pool_.end());
+ if (mru != pool_.end() && !mru->in_use) {
+ mru->in_use = true;
+ return mru->conn;
+ }
+ else {
+ return 0;
+ }
+}
//// grab //////////////////////////////////////////////////////////////
@@ -35,50 +100,15 @@
Connection*
ConnectionPool::grab()
{
- mutex_.lock();
-
- // Find first unused connection in the pool
- PoolIt it = pool_.begin();
- while ((it != pool_.end()) && it->in_use) {
- ++it;
- }
-
- if (it != pool_.end()) {
- // Found at least one unused connection. Try to find more, and
- // decide which is the most recently used.
- unsigned int tmax = max_idle_time();
- time_t now = time(0);
- PoolIt mru = it;
- ++it;
- while (it != pool_.end()) {
- if (it->in_use) {
- // Can't use this one, it's busy
- ++it;
- }
- else if ((now - it->last_used) > tmax) {
- // This one's too old; nuke it
- PoolIt doomed = it;
- ++it;
- pool_.erase(doomed);
- }
- else if (it->last_used > mru->last_used) {
- // Ah, found a free one more recently used;
hang onto it
- mru = it;
- ++it;
- }
- }
-
- mru->in_use = true;
- mutex_.unlock();
- return mru->conn;
+ ScopedLock lock(mutex_); // ensure we're not interfered with
+ remove_old_connections();
+ if (Connection* mru = find_mru()) {
+ return mru;
}
else {
- // Pool was empty when this function was called, so create and
- // return a new one.
+ // No free connections, so create and return a new one.
pool_.push_back(ConnectionInfo(create()));
- Connection* pc = pool_.back().conn;
- mutex_.unlock();
- return pc;
+ return pool_.back().conn;
}
}
@@ -88,17 +118,34 @@
void
ConnectionPool::release(const Connection* pc)
{
- mutex_.lock();
+ ScopedLock lock(mutex_); // ensure we're not interfered with
for (PoolIt it = pool_.begin(); it != pool_.end(); ++it) {
if (it->conn == pc) {
it->in_use = false;
+ it->last_used = time(0);
break;
}
}
+}
- mutex_.unlock();
+
+//// remove_old_connections ////////////////////////////////////////////
+// Remove connections that were last used too long ago.
+
+void
+ConnectionPool::remove_old_connections()
+{
+ PoolIt it = pool_.begin(), doomed;
+ TooOld<ConnectionInfo> too_old(max_idle_time());
+
+ while ((it = std::find_if(it, pool_.end(), too_old)) != pool_.end()) {
+ doomed = it++;
+ destroy(doomed->conn);
+ pool_.erase(doomed);
+ }
}
+
} // end namespace mysqlpp
Modified: trunk/lib/cpool.h
URL:
http://svn.gna.org/viewcvs/mysqlpp/trunk/lib/cpool.h?rev=2028&r1=2027&r2=2028&view=diff
==============================================================================
--- trunk/lib/cpool.h (original)
+++ trunk/lib/cpool.h Thu Dec 27 04:18:04 2007
@@ -2,9 +2,10 @@
/// \brief Declares the ConnectionPool class.
/***********************************************************************
- Copyright (c) 2007 by Educational Technology Resources, Inc.
- Others may also hold copyrights on code in this file. See the
- CREDITS file in the top directory of the distribution for details.
+ Copyright (c) 2007 by Educational Technology Resources, Inc. and
+ (c) 2007 by Jonathan Wakely. Others may also hold copyrights on
+ code in this file. See the CREDITS file in the top directory of
+ the distribution for details.
This file is part of MySQL++.
@@ -31,6 +32,7 @@
#include <list>
+#include <assert.h>
#include <time.h>
namespace mysqlpp {
@@ -63,20 +65,44 @@
ConnectionPool() { }
/// \brief Dtor
- virtual ~ConnectionPool() { }
-
- /// \brief Get the most recently used free connection in the pool.
+ ///
+ /// If this assertion is raised, it means the derived class isn't
+ /// calling clear() in its dtor.
+ virtual ~ConnectionPool() { assert(pool_.empty()); }
+
+ /// \brief Drains the pool, freeing all allocated memory.
+ ///
+ /// A derived class must call this in its dtor to avoid leaking all
+ /// Connection objects still in existence. We can't do it up at
+ /// this level because this class's dtor can't call our subclass's
+ /// destroy() method.
+ void clear();
+
+ /// \brief Grab a free connection from the pool.
///
/// This method creates a new connection if an unused one doesn't
/// exist, and destroys any that have remained unused for too long.
+ /// If there is more than one free connection, we return the most
+ /// recently used one; this allows older connections to die off over
+ /// time when the caller's need for connections decreases.
///
- /// \retval a pointer to the most recently used connection
+ /// Do not delete the returned pointer. This object manages the
+ /// lifetime of connection objects it creates.
+ ///
+ /// \retval a pointer to the connection
Connection* grab();
- /// \brief Mark the given connection as no longer in use.
+ /// \brief Return a connection to the pool
///
- /// If you don't call this function, connections returned by
- /// connection() will never go away!
+ /// Marks the connection as no longer in use. Also resets the
+ /// last-used time to the current time, so a call implies that the
+ /// connection was used shortly prior.
+ ///
+ /// This means that you must always release connections as soon as
+ /// you're done with them. Don't hold on to idle connections! If
+ /// you delay releasing them, it screws up the "most recently used"
+ /// algorithm. If you never release them, they can never be closed
+ /// when idle, so you might as well not be using a pool.
void release(const Connection* pc);
protected:
@@ -92,6 +118,15 @@
///
/// \retval A connected Connection object
virtual Connection* create() = 0;
+
+ /// \brief Destroy a connection
+ ///
+ /// Subclasses must override this.
+ ///
+ /// This is for destroying the objects returned by create().
+ /// Because we can't know what the derived class did to create the
+ /// connection we can't reliably know how to destroy it.
+ virtual void destroy(Connection*) = 0;
/// \brief Returns the maximum number of seconds a connection is
/// able to remain idle before it is dropped.
@@ -116,9 +151,26 @@
in_use(true)
{
}
+
+ // Strict weak ordering for ConnectionInfo objects.
+ //
+ // This ordering defines all in-use connections to be "less
+ // than" those not in use. Within each group, connections
+ // less recently touched are less than those more recent.
+ bool operator<(const ConnectionInfo& rhs) const
+ {
+ const ConnectionInfo& lhs = *this;
+ return lhs.in_use == rhs.in_use ?
+ lhs.last_used < rhs.last_used :
+ lhs.in_use;
+ }
};
typedef std::list<ConnectionInfo> PoolT;
typedef PoolT::iterator PoolIt;
+
+ //// Internal support functions
+ Connection* find_mru();
+ void remove_old_connections();
//// Internal data
PoolT pool_;
_______________________________________________
Mysqlpp-commits mailing list
[email protected]
https://mail.gna.org/listinfo/mysqlpp-commits
