From 8783fc35f27b8583755d53d70efbf6cece27ca29 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Wed, 14 Apr 2010 22:06:12 +0000 Subject: 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 --- libs/ardour/ardour/butler.h | 13 ++++++++ libs/ardour/ardour/session_event.h | 2 ++ libs/ardour/butler.cc | 35 ++++++++++++++++++++ libs/pbd/pbd/pool.h | 32 ++++++++++++++++--- libs/pbd/pool.cc | 65 +++++++++++++++++++++++++++++++------- 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 +#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 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::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 free_list; + RingBuffer 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 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* t) { + _trash = t; + } + + void add_to_trash (CrossThreadPool *); + private: GPrivate* _key; std::string _name; unsigned long _item_size; unsigned long _nitems; + RingBuffer* _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 #include #include +#include #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(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 (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()); +} + -- cgit v1.2.3