ardour/livetrax
13
0
Fork 0
Code

Fix mysterious crashes such as #7049

Browse Source 
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.
This commit is contained in:
Robin Gareus 2016-11-24 09:02:47 +01:00
parent e2012bc5e4
commit bc0fa4d689
2 changed files with 27 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
libs/pbd/pbd/abstract_ui.cc
View File

@ -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;
}

1
libs/pbd/pbd/abstract_ui.h
View File

@ -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);