mutex 'er up

Some overzealous locking to track down RequestObject related crashes.

bc0fa4d689a4 wrongly locked the current event loop's
request_invalidation_lock instead of the invalidation's list lock.

Also Abstract UI is able to delete requests concurrently with with
EventLoop invalidation.
e.g. PortManager::PortRegisteredOrUnregistered  and GlobalPortMatrixWindow
so the lock needs to be exposed.

If this solves various issues, mutexes should to be consolidated
(request_buffer_map_lock + request_invalidation_lock) and be chosen
such that there is as little contention as possible.

diff --git a/libs/pbd/event_loop.cc b/libs/pbd/event_loop.cc
index ea3f7a46afb5ae2b347967169c8ace80e69afc6a..fbbf9c83aab4f15b449ec7c4089080bb1bb4acd7 100644 (file)
--- a/libs/pbd/event_loop.cc
+++ b/libs/pbd/event_loop.cc
@@ -60,7 +60,7 @@ EventLoop::set_event_loop_for_thread (EventLoop* loop)
 void*
 EventLoop::invalidate_request (void* data)
 {
-        InvalidationRecord* ir = (InvalidationRecord*) data;
+       InvalidationRecord* ir = (InvalidationRecord*) data;
 
        /* Some of the requests queued with an EventLoop may involve functors
         * that make method calls to objects whose lifetime is shorter
@@ -88,6 +88,7 @@ EventLoop::invalidate_request (void* data)
 
         if (ir->event_loop) {
                Glib::Threads::Mutex::Lock lm (ir->event_loop->slot_invalidation_mutex());
+               Glib::Threads::Mutex::Lock lr (ir->event_loop->request_invalidation_mutex());
                for (list<BaseRequestObject*>::iterator i = ir->requests.begin(); i != ir->requests.end(); ++i) {
                        (*i)->valid = false;
                        (*i)->invalidation = 0;

diff --git a/libs/pbd/pbd/abstract_ui.cc b/libs/pbd/pbd/abstract_ui.cc
index 594dec34dd411f940b053396a22bc88615a4a616..ed358cb7cfba6f0245941ba710051937771e4cb9 100644 (file)
--- a/libs/pbd/pbd/abstract_ui.cc
+++ b/libs/pbd/pbd/abstract_ui.cc
@@ -240,7 +240,8 @@ AbstractUI<RequestObject>::handle_ui_requests ()
                                        request_buffer_map_lock.lock ();
                                        if (vec.buf[0]->invalidation) {
                                                DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1: removing invalidation record for that request\n", event_loop_name()));
-                                               Glib::Threads::Mutex::Lock lm (request_invalidation_lock);
+                                               assert (vec.buf[0]->invalidation->event_loop);
+                                               Glib::Threads::Mutex::Lock lm (vec.buf[0]->invalidation->event_loop->request_invalidation_mutex());
                                                if (!(*i).second->dead) {
                                                        vec.buf[0]->invalidation->requests.remove (vec.buf[0]);
                                                }
@@ -257,6 +258,7 @@ AbstractUI<RequestObject>::handle_ui_requests ()
 
        /* clean up any dead request buffers (their thread has exited) */
 
+       Glib::Threads::Mutex::Lock lr (request_invalidation_lock);
        for (i = request_buffers.begin(); i != request_buffers.end(); ) {
                if ((*i).second->dead) {
                        DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1/%2 deleting dead per-thread request buffer for %3 @ %4\n",
@@ -304,8 +306,9 @@ AbstractUI<RequestObject>::handle_ui_requests ()
                 */
 
                if (req->invalidation) {
-                       Glib::Threads::Mutex::Lock lm (request_invalidation_lock);
                        DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1/%2 remove request from its invalidation list\n", event_loop_name(), pthread_name()));
+                       assert (req->invalidation->event_loop && req->invalidation->event_loop != this);
+                       Glib::Threads::Mutex::Lock lm (req->invalidation->event_loop->request_invalidation_mutex());
 
                        /* after this call, if the object referenced by the
                         * invalidation record is deleted, it will no longer

diff --git a/libs/pbd/pbd/abstract_ui.h b/libs/pbd/pbd/abstract_ui.h
index 278f8a2603bc623d60eb02c09b20d36d6c803ea2..e23f443c25f8c55c3dda9809f8b6daebbe7c9f84 100644 (file)
--- a/libs/pbd/pbd/abstract_ui.h
+++ b/libs/pbd/pbd/abstract_ui.h
@@ -60,7 +60,8 @@ class ABSTRACT_UI_API AbstractUI : public BaseUI
 
        void register_thread (pthread_t, std::string, uint32_t num_requests);
        void call_slot (EventLoop::InvalidationRecord*, const boost::function<void()>&);
-        Glib::Threads::Mutex& slot_invalidation_mutex() { return request_buffer_map_lock; }
+       Glib::Threads::Mutex& slot_invalidation_mutex() { return request_buffer_map_lock; }
+       Glib::Threads::Mutex& request_invalidation_mutex() { return request_invalidation_lock; }
 
        Glib::Threads::Mutex request_buffer_map_lock;
        Glib::Threads::Mutex request_invalidation_lock;

diff --git a/libs/pbd/pbd/event_loop.h b/libs/pbd/pbd/event_loop.h
index b6e07b44de1308fa6dbffd747a0d0b6b6b15ed82..0001692d3dff8305ca60236f665873ce1f90d130 100644 (file)
--- a/libs/pbd/pbd/event_loop.h
+++ b/libs/pbd/pbd/event_loop.h
@@ -76,9 +76,10 @@ class LIBPBD_API EventLoop
        };
 
        virtual void call_slot (InvalidationRecord*, const boost::function<void()>&) = 0;
-        virtual Glib::Threads::Mutex& slot_invalidation_mutex() = 0;
+       virtual Glib::Threads::Mutex& slot_invalidation_mutex() = 0;
+       virtual Glib::Threads::Mutex& request_invalidation_mutex() = 0;
 
-        std::string event_loop_name() const { return _name; }
+       std::string event_loop_name() const { return _name; }
 
        static EventLoop* get_event_loop_for_thread();
        static void set_event_loop_for_thread (EventLoop* ui);

diff --git a/session_utils/common.cc b/session_utils/common.cc
index 8d1cdec950d8afb39c3b9c8650701b23ef9b6efb..61a3a69f65a1dd809ed5c03cb67158d3342886f3 100644 (file)
--- a/session_utils/common.cc
+++ b/session_utils/common.cc
@@ -77,10 +77,12 @@ class MyEventLoop : public sigc::trackable, public EventLoop
                }
 
                Glib::Threads::Mutex& slot_invalidation_mutex() { return request_buffer_map_lock; }
+               Glib::Threads::Mutex& request_invalidation_mutex() { return request_invalidation_lock; }
 
        private:
                Glib::Threads::Thread* run_loop_thread;
                Glib::Threads::Mutex   request_buffer_map_lock;
+               Glib::Threads::Mutex   request_invalidation_lock;
 };
 
 static MyEventLoop *event_loop;

diff --git a/tools/luadevel/luasession.cc b/tools/luadevel/luasession.cc
index a131bb443f799b5720119ead85a0a42a9e3f75ec..649346b3f773738c8baac2b6f5476f42a5b266a2 100644 (file)
--- a/tools/luadevel/luasession.cc
+++ b/tools/luadevel/luasession.cc
@@ -109,10 +109,12 @@ class MyEventLoop : public sigc::trackable, public EventLoop
                }
 
                Glib::Threads::Mutex& slot_invalidation_mutex () { return request_buffer_map_lock; }
+               Glib::Threads::Mutex& request_invalidation_mutex() { return request_invalidation_lock; }
 
        private:
                Glib::Threads::Thread* run_loop_thread;
                Glib::Threads::Mutex   request_buffer_map_lock;
+               Glib::Threads::Mutex   request_invalidation_lock;
 };
 
 static MyEventLoop *event_loop = NULL;