mutex 'er up

Some overzealous locking to track down RequestObject related crashes.

bc0fa4d689 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.
This commit is contained in:
Robin Gareus 2016-12-13 23:46:55 +01:00
parent 176625d9e0
commit fa07233a17
6 changed files with 16 additions and 6 deletions

View File

@ -60,7 +60,7 @@ EventLoop::set_event_loop_for_thread (EventLoop* loop)
void* void*
EventLoop::invalidate_request (void* data) EventLoop::invalidate_request (void* data)
{ {
InvalidationRecord* ir = (InvalidationRecord*) data; InvalidationRecord* ir = (InvalidationRecord*) data;
/* Some of the requests queued with an EventLoop may involve functors /* Some of the requests queued with an EventLoop may involve functors
* that make method calls to objects whose lifetime is shorter * that make method calls to objects whose lifetime is shorter
@ -88,6 +88,7 @@ EventLoop::invalidate_request (void* data)
if (ir->event_loop) { if (ir->event_loop) {
Glib::Threads::Mutex::Lock lm (ir->event_loop->slot_invalidation_mutex()); 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) { for (list<BaseRequestObject*>::iterator i = ir->requests.begin(); i != ir->requests.end(); ++i) {
(*i)->valid = false; (*i)->valid = false;
(*i)->invalidation = 0; (*i)->invalidation = 0;

View File

@ -240,7 +240,8 @@ AbstractUI<RequestObject>::handle_ui_requests ()
request_buffer_map_lock.lock (); request_buffer_map_lock.lock ();
if (vec.buf[0]->invalidation) { if (vec.buf[0]->invalidation) {
DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1: removing invalidation record for that request\n", event_loop_name())); 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) { if (!(*i).second->dead) {
vec.buf[0]->invalidation->requests.remove (vec.buf[0]); 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) */ /* 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(); ) { for (i = request_buffers.begin(); i != request_buffers.end(); ) {
if ((*i).second->dead) { if ((*i).second->dead) {
DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1/%2 deleting dead per-thread request buffer for %3 @ %4\n", 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) { 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())); 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 /* after this call, if the object referenced by the
* invalidation record is deleted, it will no longer * invalidation record is deleted, it will no longer

View File

@ -60,7 +60,8 @@ class ABSTRACT_UI_API AbstractUI : public BaseUI
void register_thread (pthread_t, std::string, uint32_t num_requests); void register_thread (pthread_t, std::string, uint32_t num_requests);
void call_slot (EventLoop::InvalidationRecord*, const boost::function<void()>&); 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_buffer_map_lock;
Glib::Threads::Mutex request_invalidation_lock; Glib::Threads::Mutex request_invalidation_lock;

View File

@ -76,9 +76,10 @@ class LIBPBD_API EventLoop
}; };
virtual void call_slot (InvalidationRecord*, const boost::function<void()>&) = 0; 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 EventLoop* get_event_loop_for_thread();
static void set_event_loop_for_thread (EventLoop* ui); static void set_event_loop_for_thread (EventLoop* ui);

View File

@ -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& slot_invalidation_mutex() { return request_buffer_map_lock; }
Glib::Threads::Mutex& request_invalidation_mutex() { return request_invalidation_lock; }
private: private:
Glib::Threads::Thread* run_loop_thread; Glib::Threads::Thread* run_loop_thread;
Glib::Threads::Mutex request_buffer_map_lock; Glib::Threads::Mutex request_buffer_map_lock;
Glib::Threads::Mutex request_invalidation_lock;
}; };
static MyEventLoop *event_loop; static MyEventLoop *event_loop;

View File

@ -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& slot_invalidation_mutex () { return request_buffer_map_lock; }
Glib::Threads::Mutex& request_invalidation_mutex() { return request_invalidation_lock; }
private: private:
Glib::Threads::Thread* run_loop_thread; Glib::Threads::Thread* run_loop_thread;
Glib::Threads::Mutex request_buffer_map_lock; Glib::Threads::Mutex request_buffer_map_lock;
Glib::Threads::Mutex request_invalidation_lock;
}; };
static MyEventLoop *event_loop = NULL; static MyEventLoop *event_loop = NULL;