Patch submitted to qpid-dev by ksmith@redhat.com. Fixes concurrency issues arising from previous move to use singleton apr pool.

"My patch does three things:

1) Modifies the APRPool class to use alloc/free semantics for APR memory pools. Each time a caller calls APRPool::get() they'll their own pool reference. I've fixed up all the call sites I can find to also call APRPool::free() at the appropriate time.

2) Caches freed APR memory pools in a STL stack. This cuts down on the number of memory pools created overall.

3) As a result of doing #1 and #2 I've introduced a guard mutex around APRPool::get() and APRPool::free(). This is to prevent concurrent access to the memory pool cache. If it's too heavyweight, the mutex along with the caching mechanism could be removed entirely."



git-svn-id: https://svn.apache.org/repos/asf/incubator/qpid/branches/M2@540511 13f79535-47bb-0310-9956-ffa450edef68

1 files changed, 38 insertions, 1 deletions

diff --git a/cpp/lib/common/sys/apr/APRPool.cpp b/cpp/lib/common/sys/apr/APRPool.cpp
index e8b71f6e8a..91481faf09 100644
--- a/cpp/lib/common/sys/apr/APRPool.cpp
+++ b/cpp/lib/common/sys/apr/APRPool.cpp
@@ -22,20 +22,57 @@
 #include "APRPool.h"
 #include "APRBase.h"
 #include <boost/pool/detail/singleton.hpp>
+#include <iostream>
+#include <sstream>
+
 
 using namespace qpid::sys;
 
 APRPool::APRPool(){
     APRBase::increment();
+    allocated_pools = new std::stack<apr_pool_t*>();    
     CHECK_APR_SUCCESS(apr_pool_create(&pool, NULL));
+    CHECK_APR_SUCCESS(apr_thread_mutex_create(&poolGuard, APR_THREAD_MUTEX_NESTED, pool));
 }
 
 APRPool::~APRPool(){
+    while(allocated_pools->size() > 0) {
+        apr_pool_t* pool = allocated_pools->top();
+        allocated_pools->pop();
+        apr_pool_destroy(pool);
+    }
     apr_pool_destroy(pool);
+    apr_thread_mutex_destroy(poolGuard);
+    delete allocated_pools;
     APRBase::decrement();
 }
 
+void APRPool::free_pool(apr_pool_t* pool) {
+    CHECK_APR_SUCCESS(apr_thread_mutex_lock(poolGuard));
+    allocated_pools->push(pool);
+    CHECK_APR_SUCCESS(apr_thread_mutex_unlock(poolGuard));
+}
+
+apr_pool_t* APRPool::allocate_pool() {
+    CHECK_APR_SUCCESS(apr_thread_mutex_lock(poolGuard));
+    apr_pool_t* retval;
+    if (allocated_pools->size() == 0) {
+        CHECK_APR_SUCCESS(apr_pool_create(&retval, pool));
+    }
+    else {
+        retval = allocated_pools->top();
+        allocated_pools->pop();
+    }
+    CHECK_APR_SUCCESS(apr_thread_mutex_unlock(poolGuard));
+    return retval;    
+}
+
 apr_pool_t* APRPool::get() {
-    return boost::details::pool::singleton_default<APRPool>::instance().pool;
+    return 
+        boost::details::pool::singleton_default<APRPool>::instance().allocate_pool(); 
+}
+
+void APRPool::free(apr_pool_t* pool) {    
+    boost::details::pool::singleton_default<APRPool>::instance().free_pool(pool);
 }