Fix mysterious crashes such as #7049

Fixes an issue with corrupted std::lists<> due to concurrent writes
to the invalidation list which eventually resulted in
EventLoop::invalidate_request() not invalidating requests.
Concurrency sucks rocks hard.

diff --git a/libs/pbd/pbd/abstract_ui.cc b/libs/pbd/pbd/abstract_ui.cc
index 6f03f2554e6b257e7c37f0db74f6de73a28534ab..5b289c68ea4edad72c70c6a20a0d2cec847b1d58 100644 (file)
--- a/libs/pbd/pbd/abstract_ui.cc
+++ b/libs/pbd/pbd/abstract_ui.cc
@@ -240,6 +240,7 @@ 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);
                                                vec.buf[0]->invalidation->requests.remove (vec.buf[0]);
                                        } else {
                                                DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1: no invalidation record for that request\n", event_loop_name()));
@@ -301,6 +302,7 @@ 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()));
 
                        /* after this call, if the object referenced by the
@@ -309,6 +311,17 @@ AbstractUI<RequestObject>::handle_ui_requests ()
                         */
 
                        req->invalidation->requests.remove (req);
+
+                       /* req->invalidation->requests is identical to
+                        * EventLoop::invalidate_request()'s ir->requests.
+                        *
+                        * However EventLoop::invalidate_request() cannot
+                        * run in parallel to this, because sigc slots are always
+                        * called in the same thread as signal emission (this event-loop).
+                        * So we do not need to expose "request_invalidation_lock",
+                        * and only provide a safeguard for modifications to the
+                        * container itself.
+                        */
                }
 
                /* at this point, an object involved in a functor could be
@@ -434,6 +447,19 @@ AbstractUI<RequestObject>::call_slot (InvalidationRecord* invalidation, const bo
        req->invalidation = invalidation;
 
        if (invalidation) {
+               /* We're about to modify a std::list<> container, this must not
+                * happen concurrently with other container modifications
+                * (invalidation->requests.remove in handle_ui_requests).
+                *
+                * Let's take a dedicated lock that is less contended than
+                * request_buffer_map_lock.
+                *
+                * This works because object's d'tor disconnect signals
+                * (member variable) before the sigc::trackable's destroy_notify
+                * runs. So there is no race between signal-emissions/call_slot()
+                * and adding new requests after the object has been invalidated.
+                */
+               Glib::Threads::Mutex::Lock lm (request_invalidation_lock);
                invalidation->requests.push_back (req);
                invalidation->event_loop = this;
        }

diff --git a/libs/pbd/pbd/abstract_ui.h b/libs/pbd/pbd/abstract_ui.h
index 78a337fc40bcf601045120ec21724b4954c5bcdd..278f8a2603bc623d60eb02c09b20d36d6c803ea2 100644 (file)
--- a/libs/pbd/pbd/abstract_ui.h
+++ b/libs/pbd/pbd/abstract_ui.h
@@ -63,6 +63,7 @@ class ABSTRACT_UI_API AbstractUI : public BaseUI
         Glib::Threads::Mutex& slot_invalidation_mutex() { return request_buffer_map_lock; }
 
        Glib::Threads::Mutex request_buffer_map_lock;
+       Glib::Threads::Mutex request_invalidation_lock;
 
        static void* request_buffer_factory (uint32_t num_requests);