Suspend deletion of cross-thread pools until they are empty. Prevents crashes when the freeze thread completes.

git-svn-id: svn://localhost/ardour2/branches/3.0@6893 d708f5d6-7413-0410-9779-e7cbd77b26cf

5 files changed, 132 insertions, 15 deletions

diff --git a/libs/ardour/ardour/butler.h b/libs/ardour/ardour/butler.h
index 60697517fb..e9e832783b 100644
--- a/libs/ardour/ardour/butler.h
+++ b/libs/ardour/ardour/butler.h
@@ -22,11 +22,20 @@
 
 #include <glibmm/thread.h>
 
+#include "pbd/ringbuffer.h"
+#include "pbd/pool.h"
 #include "ardour/types.h"
 #include "ardour/session_handle.h"
 
 namespace ARDOUR {
 
+/**
+ *  One of the Butler's functions is to clean up (ie delete) unused CrossThreadPools.
+ *  When a thread with a CrossThreadPool terminates, its CTP is added to pool_trash.
+ *  When the Butler thread wakes up, we check this trash buffer for CTPs, and if they
+ *  are empty they are deleted.
+ */
+
 class Butler : public SessionHandleRef
 {
   public:
@@ -67,6 +76,10 @@ class Butler : public SessionHandleRef
 	int          request_pipe[2];
 	uint32_t     audio_dstream_buffer_size;
 	uint32_t     midi_dstream_buffer_size;
+	RingBuffer<CrossThreadPool*> pool_trash;
+
+private:
+	void empty_pool_trash ();
 };
 
 } // namespace ARDOUR
diff --git a/libs/ardour/ardour/session_event.h b/libs/ardour/ardour/session_event.h
index 882142c68d..2306bb7c1a 100644
--- a/libs/ardour/ardour/session_event.h
+++ b/libs/ardour/ardour/session_event.h
@@ -117,6 +117,8 @@ struct SessionEvent {
 private:
     static PerThreadPool* pool;
     CrossThreadPool* own_pool;
+
+    friend class Butler;
 };
 
 class SessionEventManager {
diff --git a/libs/ardour/butler.cc b/libs/ardour/butler.cc
index 571ac5f883..8b88197073 100644
--- a/libs/ardour/butler.cc
+++ b/libs/ardour/butler.cc
@@ -43,8 +43,10 @@ Butler::Butler(Session& s)
 	, thread(0)
 	, audio_dstream_buffer_size(0)
 	, midi_dstream_buffer_size(0)
+	, pool_trash(16)
 {
 	g_atomic_int_set(&should_do_transport_work, 0);
+	SessionEvent::pool->set_trash (&pool_trash);
 }
 
 Butler::~Butler()
@@ -331,6 +333,8 @@ Butler::thread_work ()
 
 			paused.signal();
 		}
+
+		empty_pool_trash ();
 	}
 
 	pthread_exit_pbd (0);
@@ -394,4 +398,35 @@ Butler::write_data_rate () const
 	return _write_data_rate > 10485.7600000f ? 0.0f : _write_data_rate;
 }
 
+void
+Butler::empty_pool_trash ()
+{
+	/* look in the trash, deleting empty pools until we come to one that is not empty */
+	
+	RingBuffer<CrossThreadPool*>::rw_vector vec;
+	pool_trash.get_read_vector (&vec);
+
+	guint deleted = 0;
+	
+	for (int i = 0; i < 2; ++i) {
+		for (guint j = 0; j < vec.len[i]; ++j) {
+			if (vec.buf[i][j]->empty()) {
+				delete vec.buf[i][j];
+				++deleted;
+			} else {
+				/* found a non-empty pool, so stop deleting */
+				if (deleted) {
+					pool_trash.increment_read_idx (deleted);
+				}
+				return;
+			}
+		}
+	}
+
+	if (deleted) {
+		pool_trash.increment_read_idx (deleted);
+	}
+}
+
 } // namespace ARDOUR
+
diff --git a/libs/pbd/pbd/pool.h b/libs/pbd/pbd/pool.h
index e8ad780dc2..2d6111a4a3 100644
--- a/libs/pbd/pbd/pool.h
+++ b/libs/pbd/pbd/pool.h
@@ -27,6 +27,9 @@
 
 #include "pbd/ringbuffer.h"
 
+/** A pool of data items that can be allocated, read from and written to
+ *  without system memory allocation or locking.
+ */
 class Pool 
 {
   public:
@@ -39,11 +42,11 @@ class Pool
 	std::string name() const { return _name; }
 
   protected:
-	RingBuffer<void*> free_list;
+	RingBuffer<void*> free_list; ///< a list of pointers to free items within block
 	std::string _name;
 
   private:
-	void *block;
+	void *block; ///< data storage area
 };
 
 class SingleAllocMultiReleasePool : public Pool
@@ -73,18 +76,31 @@ class MultiAllocSingleReleasePool : public Pool
     Glib::Mutex* m_lock;
 };
 
+class PerThreadPool;
+
+/** A per-thread pool of data */
 class CrossThreadPool : public Pool
 {
   public:
-	CrossThreadPool (std::string n, unsigned long isize, unsigned long nitems);
+	CrossThreadPool (std::string n, unsigned long isize, unsigned long nitems, PerThreadPool *);
 
 	void* alloc ();
 	void push (void *);
+
+	PerThreadPool* parent () const {
+		return _parent;
+	}
+
+	bool empty ();
 	
   private:
 	RingBuffer<void*> pending;
+	PerThreadPool* _parent;
 };
 
+/** A class to manage per-thread pools of memory.  One object of this class is instantiated,
+ *  and then it is used to create per-thread pools as required.
+ */
 class PerThreadPool
 {
   public:
@@ -94,12 +110,20 @@ class PerThreadPool
 
 	void  create_per_thread_pool (std::string name, unsigned long item_size, unsigned long nitems);
 	CrossThreadPool* per_thread_pool ();
-	
+
+	void set_trash (RingBuffer<CrossThreadPool*>* t) {
+		_trash = t;
+	}
+
+	void add_to_trash (CrossThreadPool *);
+
   private:
 	GPrivate* _key;
 	std::string _name;
 	unsigned long _item_size;
 	unsigned long _nitems;
+	RingBuffer<CrossThreadPool*>* _trash;
+	Glib::Mutex _trash_write_mutex;
 };
 
 #endif // __qm_pool_h__
diff --git a/libs/pbd/pool.cc b/libs/pbd/pool.cc
index 9c60e5c4ce..7859408c45 100644
--- a/libs/pbd/pool.cc
+++ b/libs/pbd/pool.cc
@@ -22,6 +22,7 @@
 #include <iostream>
 #include <vector>
 #include <cstdlib>
+#include <cassert>
 
 #include "pbd/pool.h"
 #include "pbd/error.h"
@@ -57,13 +58,14 @@ Pool::~Pool ()
 	free (block);
 }
 
+/** Allocate an item's worth of memory in the Pool by taking one from the free list.
+ *  @return Pointer to free item.
+ */
 void *
 Pool::alloc ()
 {
 	void *ptr;
 
-//	cerr << _name << " pool " << " alloc, thread = " << pthread_name() << " space = " << free_list->read_space() << endl;
-
 	if (free_list.read (&ptr, 1) < 1) {
 		fatal << "CRITICAL: " << _name << " POOL OUT OF MEMORY - RECOMPILE WITH LARGER SIZE!!" << endmsg;
 		/*NOTREACHED*/
@@ -73,18 +75,18 @@ Pool::alloc ()
 	}
 }
 
+/** Release an item's memory by writing its location to the free list */
 void		
 Pool::release (void *ptr)
 {
 	free_list.write (&ptr, 1);
-//	cerr << _name << ": release, now has " << free_list->read_space() << endl;
 }
 
 /*---------------------------------------------*/
 
 MultiAllocSingleReleasePool::MultiAllocSingleReleasePool (string n, unsigned long isize, unsigned long nitems) 
-	: Pool (n, isize, nitems),
-        m_lock(0)
+	: Pool (n, isize, nitems)
+	, m_lock(0)
 {
 }
 
@@ -94,8 +96,8 @@ MultiAllocSingleReleasePool::~MultiAllocSingleReleasePool ()
 }
 
 SingleAllocMultiReleasePool::SingleAllocMultiReleasePool (string n, unsigned long isize, unsigned long nitems) 
-	: Pool (n, isize, nitems),
-    m_lock(0)
+	: Pool (n, isize, nitems)
+	, m_lock(0)
 {
 }
 
@@ -148,11 +150,18 @@ SingleAllocMultiReleasePool::release (void* ptr)
 static void 
 free_per_thread_pool (void* ptr)
 {
-	Pool* pptr = static_cast<Pool*>(ptr);
-	delete pptr;
+	/* Rather than deleting the CrossThreadPool now, we add it to our trash buffer.
+	 * This prevents problems if other threads still require access to this CrossThreadPool.
+	 * We assume that some other agent will clean out the trash buffer as required.
+	 */
+	CrossThreadPool* cp = static_cast<CrossThreadPool*> (ptr);
+	assert (cp);
+
+	cp->parent()->add_to_trash (cp);
 }
  
 PerThreadPool::PerThreadPool ()
+	: _trash (0)
 {
 	{
 		/* for some reason this appears necessary to get glib's thread private stuff to work */
@@ -163,13 +172,21 @@ PerThreadPool::PerThreadPool ()
 	_key = g_private_new (free_per_thread_pool);
 }
 
+/** Create a new CrossThreadPool and set the current thread's private _key to point to it.
+ *  @param n Name.
+ *  @param isize Size of each item in the pool.
+ *  @param nitems Number of items in the pool.
+ */
 void
 PerThreadPool::create_per_thread_pool (string n, unsigned long isize, unsigned long nitems)
 {
-	Pool* p = new CrossThreadPool (n, isize, nitems);
+	CrossThreadPool* p = new CrossThreadPool (n, isize, nitems, this);
 	g_private_set (_key, p);
 }
 
+/** @return CrossThreadPool for the current thread, which must previously have been created by
+ *  calling create_per_thread_pool in the current thread.
+ */
 CrossThreadPool*
 PerThreadPool::per_thread_pool ()
 {
@@ -181,9 +198,27 @@ PerThreadPool::per_thread_pool ()
 	return p;
 }
 
-CrossThreadPool::CrossThreadPool  (string n, unsigned long isize, unsigned long nitems)
+/** Add a CrossThreadPool to our trash, if we have one.  If not, a warning is emitted. */
+void
+PerThreadPool::add_to_trash (CrossThreadPool* p)
+{
+	if (!_trash) {
+		warning << "Pool " << p->name() << " has no trash collector; a memory leak has therefore occurred" << endmsg;
+		return;
+	}
+
+	/* we have a lock here so that multiple threads can safely call add_to_trash (even though there
+	   can only be one writer to the _trash RingBuffer)
+	*/
+		
+	Glib::Mutex::Lock lm (_trash_write_mutex);
+	_trash->write (&p, 1);
+}
+
+CrossThreadPool::CrossThreadPool  (string n, unsigned long isize, unsigned long nitems, PerThreadPool* p)
 	: Pool (n, isize, nitems)
 	, pending (nitems)
+	, _parent (p)
 {
 	
 }
@@ -203,3 +238,11 @@ CrossThreadPool::push (void* t)
 {
 	pending.write (&t, 1);
 }
+
+/** @return true if there is nothing in this pool */
+bool
+CrossThreadPool::empty ()
+{
+	return (free_list.write_space() == pending.read_space());
+}
+