Suspend deletion of cross-thread pools until they are empty. Prevents crashes when...
authorCarl Hetherington <carl@carlh.net>
Wed, 14 Apr 2010 22:06:12 +0000 (22:06 +0000)
committerCarl Hetherington <carl@carlh.net>
Wed, 14 Apr 2010 22:06:12 +0000 (22:06 +0000)
git-svn-id: svn://localhost/ardour2/branches/3.0@6893 d708f5d6-7413-0410-9779-e7cbd77b26cf

libs/ardour/ardour/butler.h
libs/ardour/ardour/session_event.h
libs/ardour/butler.cc
libs/pbd/pbd/pool.h
libs/pbd/pool.cc

index 60697517fb4095bb20cf86456b7d967dc231aad6..e9e832783beda09e3653df92a87755df57383edc 100644 (file)
 
 #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
index 882142c68d1916d9dfd0ad60e1f1a991d7c2b049..2306bb7c1a6c3628ce7192e1fbc45f1be301133d 100644 (file)
@@ -117,6 +117,8 @@ struct SessionEvent {
 private:
     static PerThreadPool* pool;
     CrossThreadPool* own_pool;
+
+    friend class Butler;
 };
 
 class SessionEventManager {
index 571ac5f8835dc1a31072f968d1826c10d299bd0b..8b88197073c10f9d50889dfb5f9f9c6d58364eed 100644 (file)
@@ -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
+
index e8ad780dc2a6052a115d4dabc5f1569358a7638f..2d6111a4a35d69594a7254d1b1bedb8a51ef97e0 100644 (file)
@@ -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__
index 9c60e5c4ced464b235e3d3ac678df476dcf3144e..7859408c45b73c87965071cecc2dd95a3b1bcc1b 100644 (file)
@@ -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());
+}
+