diff --git a/libs/ardour/ardour/control_protocol_manager.h b/libs/ardour/ardour/control_protocol_manager.h index ef7541890a..1eee1e77bc 100644 --- a/libs/ardour/ardour/control_protocol_manager.h +++ b/libs/ardour/ardour/control_protocol_manager.h @@ -72,7 +72,6 @@ class LIBARDOUR_API ControlProtocolManager : public PBD::Stateful, public ARDOUR void load_mandatory_protocols (); void midi_connectivity_established (); void drop_protocols (); - void register_request_buffer_factories (); int activate (ControlProtocolInfo&); int deactivate (ControlProtocolInfo&); diff --git a/libs/ardour/control_protocol_manager.cc b/libs/ardour/control_protocol_manager.cc index 28971dbc79..6676e829c7 100644 --- a/libs/ardour/control_protocol_manager.cc +++ b/libs/ardour/control_protocol_manager.cc @@ -577,24 +577,6 @@ ControlProtocolManager::midi_connectivity_established () } } -void -ControlProtocolManager::register_request_buffer_factories () -{ - Glib::Threads::RWLock::ReaderLock lm (protocols_lock); - - for (list::iterator i = control_protocol_info.begin(); i != control_protocol_info.end(); ++i) { - - if ((*i)->descriptor == 0) { - warning << string_compose (_("Control protocol \"%1\" has no descriptor"), (*i)->name) << endmsg; - continue; - } - - if ((*i)->descriptor->request_buffer_factory) { - EventLoop::register_request_buffer_factory ((*i)->descriptor->name, (*i)->descriptor->request_buffer_factory); - } - } -} - void ControlProtocolManager::stripable_selection_changed (StripableNotificationListPtr sp) { diff --git a/libs/ardour/globals.cc b/libs/ardour/globals.cc index af39dac1bc..bd0cea4d92 100644 --- a/libs/ardour/globals.cc +++ b/libs/ardour/globals.cc @@ -651,20 +651,6 @@ ARDOUR::init (bool try_optimization, const char* localedir, bool with_gui) ControlProtocolManager::instance ().discover_control_protocols (); - /* for each control protocol, check for a request buffer factory method - and if it exists, store it in the EventLoop list of such - methods. This allows the relevant threads to register themselves - with EventLoops so that signal emission can be RT-safe. - */ - - ControlProtocolManager::instance ().register_request_buffer_factories (); - /* it would be nice if this could auto-register itself in the - constructor, since MidiControlUI is a singleton, but it can't be - created until after the engine is running. Therefore we have to - explicitly register it here. - */ - EventLoop::register_request_buffer_factory (X_("midiUI"), MidiControlUI::request_factory); - /* Every Process Graph thread (up to hardware_concurrency) keeps a buffer. * The main engine callback uses one (but returns it after use * each cycle). Session Export uses one, and the GUI requires diff --git a/libs/gtkmm2ext/gtk_ui.cc b/libs/gtkmm2ext/gtk_ui.cc index fa042ab2c3..afac25e5a2 100644 --- a/libs/gtkmm2ext/gtk_ui.cc +++ b/libs/gtkmm2ext/gtk_ui.cc @@ -106,10 +106,6 @@ UI::UI (string application_name, string thread_name, int *argc, char ***argv) set_event_loop_for_thread (this); - /* we will be receiving requests */ - - EventLoop::register_request_buffer_factory ("gui", request_buffer_factory); - /* attach our request source to the default main context */ attach_request_source (); diff --git a/libs/pbd/event_loop.cc b/libs/pbd/event_loop.cc index 966484d93c..f8ee665203 100644 --- a/libs/pbd/event_loop.cc +++ b/libs/pbd/event_loop.cc @@ -37,7 +37,7 @@ static void do_not_delete_the_loop_pointer (void*) { } Glib::Threads::Private EventLoop::thread_event_loop (do_not_delete_the_loop_pointer); -Glib::Threads::RWLock EventLoop::thread_buffer_requests_lock; +Glib::Threads::Mutex EventLoop::thread_buffer_requests_lock; EventLoop::ThreadRequestBufferList EventLoop::thread_buffer_requests; EventLoop::RequestBufferSuppliers EventLoop::request_buffer_suppliers; @@ -113,16 +113,13 @@ vector EventLoop::get_request_buffers_for_target_thread (const std::string& target_thread) { vector ret; - Glib::Threads::RWLock::WriterLock lm (thread_buffer_requests_lock); + Glib::Threads::Mutex::Lock lm (thread_buffer_requests_lock); DEBUG_TRACE (PBD::DEBUG::EventLoop, string_compose ("%1 look for request buffers via %2\n", pthread_name(), target_thread)); - for (ThreadRequestBufferList::const_iterator x = thread_buffer_requests.begin(); x != thread_buffer_requests.end(); ++x) { - - if (x->second.target_thread_name == target_thread) { - DEBUG_TRACE (PBD::DEBUG::EventLoop, string_compose ("for thread \"%1\", request buffer for %2/%3 thread %4\n", target_thread, x->first, x->second.emitting_thread)); - ret.push_back (x->second); - } + for (auto const & tbr : thread_buffer_requests) { + DEBUG_TRACE (PBD::DEBUG::EventLoop, string_compose ("for thread \"%1\", request buffer for %2 (%3) thread %4\n", target_thread, tbr.emitting_thread, tbr.num_requests)); + ret.push_back (tbr); } DEBUG_TRACE (PBD::DEBUG::EventLoop, string_compose ("for thread \"%1\", found %2 request buffers\n", target_thread, ret.size())); @@ -130,21 +127,6 @@ EventLoop::get_request_buffers_for_target_thread (const std::string& target_thre return ret; } -void -EventLoop::register_request_buffer_factory (const string& target_thread_name, void* (*factory)(uint32_t)) -{ - - RequestBufferSupplier trs; - trs.name = target_thread_name; - trs.factory = factory; - - { - Glib::Threads::RWLock::WriterLock lm (thread_buffer_requests_lock); - request_buffer_suppliers.push_back (trs); - } - DEBUG_TRACE (PBD::DEBUG::EventLoop, string_compose ("event loop %1 registered a buffer factory for %2\n", pthread_name(), target_thread_name)); -} - void EventLoop::pre_register (const string& emitting_thread_name, uint32_t num_requests) { @@ -160,85 +142,56 @@ EventLoop::pre_register (const string& emitting_thread_name, uint32_t num_reques */ ThreadBufferMapping mapping; - Glib::Threads::RWLock::WriterLock lm (thread_buffer_requests_lock); + Glib::Threads::Mutex::Lock lm (thread_buffer_requests_lock); - for (RequestBufferSuppliers::iterator trs = request_buffer_suppliers.begin(); trs != request_buffer_suppliers.end(); ++trs) { + mapping.emitting_thread = pthread_self(); + mapping.num_requests = num_requests; - if (!trs->factory) { - /* no factory - no request buffer required or expected */ - continue; - } + /* now store it where the receiving thread (trs->name) can find + it if and when it is created. (Discovery happens in the + AbstractUI constructor. Note that if + */ - if (emitting_thread_name == trs->name) { - /* no need to register an emitter with itself */ - continue; - } + /* management of the thread_request_buffers map works as + * follows: + * + * An entry will remain in the map after the thread exits. + * + * The receiving thread may (if it receives requests from other + * threads) notice the dead buffer. If it does, it will delete + * the request buffer, and call + * ::remove_request_buffer_from_map() to get rid of it from the map. + * + * This does mean that the lifetime of the request buffer is + * indeterminate: if the receiving thread were to receive no + * further requests, the request buffer will live on + * forever. But this is OK, because if there are no requests + * arriving, the receiving thread is not attempting to use the + * request buffer(s) in any way. + * + * Note, however, that *if* an emitting thread is recreated + * with the same name (e.g. when a control surface is + * enabled/disabled/enabled), then the request buffer for the + * new thread will replace the map entry for the key, because + * of the matching thread names. This does mean that + * potentially the request buffer can leak in this case, but + * (a) these buffers are not really that large anyway (b) the + * scenario is not particularly common (c) the buffers would + * typically last across a session instance if not program + * lifetime anyway. + */ - mapping.emitting_thread = pthread_self(); - mapping.target_thread_name = trs->name; - - /* Allocate a suitably sized request buffer. This will set the - * thread-local variable that holds a pointer to this request - * buffer. - */ - mapping.request_buffer = trs->factory (num_requests); - - /* now store it where the receiving thread (trs->name) can find - it if and when it is created. (Discovery happens in the - AbstractUI constructor. Note that if - */ - - const string key = string_compose ("%1/%2", emitting_thread_name, mapping.target_thread_name); - - /* management of the thread_request_buffers map works as - * follows: - * - * when the factory method was called above, the pointer to the - * created buffer is set as a thread-local-storage (TLS) value - * for this (the emitting) thread. - * - * The TLS value is set up with a destructor that marks the - * request buffer as "dead" when the emitting thread exits. - * - * An entry will remain in the map after the thread exits. - * - * The receiving thread may (if it receives requests from other - * threads) notice the dead buffer. If it does, it will delete - * the request buffer, and call - * ::remove_request_buffer_from_map() to get rid of it from the map. - * - * This does mean that the lifetime of the request buffer is - * indeterminate: if the receiving thread were to receive no - * further requests, the request buffer will live on - * forever. But this is OK, because if there are no requests - * arriving, the receiving thread is not attempting to use the - * request buffer(s) in any way. - * - * Note, however, that *if* an emitting thread is recreated - * with the same name (e.g. when a control surface is - * enabled/disabled/enabled), then the request buffer for the - * new thread will replace the map entry for the key, because - * of the matching thread names. This does mean that - * potentially the request buffer can leak in this case, but - * (a) these buffers are not really that large anyway (b) the - * scenario is not particularly common (c) the buffers would - * typically last across a session instance if not program - * lifetime anyway. - */ - - thread_buffer_requests[key] = mapping; - DEBUG_TRACE (PBD::DEBUG::EventLoop, string_compose ("pre-registered request buffer for \"%1\" to send to \"%2\", buffer @ %3 (key was %4)\n", - emitting_thread_name, trs->name, mapping.request_buffer, key)); - } + thread_buffer_requests.push_back (mapping); + DEBUG_TRACE (PBD::DEBUG::EventLoop, string_compose ("pre-registered thread \"%1\"\n", emitting_thread_name)); } void -EventLoop::remove_request_buffer_from_map (void* ptr) +EventLoop::remove_request_buffer_from_map (pthread_t pth) { - Glib::Threads::RWLock::WriterLock lm (thread_buffer_requests_lock); + Glib::Threads::Mutex::Lock lm (thread_buffer_requests_lock); for (ThreadRequestBufferList::iterator x = thread_buffer_requests.begin(); x != thread_buffer_requests.end(); ++x) { - if (x->second.request_buffer == ptr) { + if (x->emitting_thread == pth) { thread_buffer_requests.erase (x); break; } diff --git a/libs/pbd/pbd/abstract_ui.cc b/libs/pbd/pbd/abstract_ui.cc index c8e9f09a47..aa6126615c 100644 --- a/libs/pbd/pbd/abstract_ui.cc +++ b/libs/pbd/pbd/abstract_ui.cc @@ -87,9 +87,11 @@ AbstractUI::AbstractUI (const string& name) { Glib::Threads::RWLock::WriterLock rbml (request_buffer_map_lock); - for (vector::iterator t = tbm.begin(); t != tbm.end(); ++t) { - request_buffers[t->emitting_thread] = static_cast (t->request_buffer); - DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%6: %1/%2/%3 create pre-registered request buffer-A @ %4 for %5\n", event_loop_name(), pthread_name(), pthread_self(), t->request_buffer, t->emitting_thread, this)); + for (auto const & t : tbm) { + request_buffers[t.emitting_thread] = new RequestBuffer (t.num_requests); + DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%6: %1/%2/%3 create pre-registered request buffer-A @ %4 for %5\n", + event_loop_name(), pthread_name(), pthread_self(), + request_buffers[t.emitting_thread], t.emitting_thread, this)); } } } @@ -97,12 +99,6 @@ AbstractUI::AbstractUI (const string& name) template AbstractUI::~AbstractUI () { - for (RequestBufferMapIterator i = request_buffers.begin(); i != request_buffers.end(); ++i) { - if ((*i).second->dead) { - EventLoop::remove_request_buffer_from_map ((*i).second); - delete (*i).second; - } - } } template void @@ -326,7 +322,7 @@ AbstractUI::handle_ui_requests () RequestBufferMapIterator tmp = i; ++tmp; /* remove it from the EventLoop static map of all request buffers */ - EventLoop::remove_request_buffer_from_map ((*i).second); + EventLoop::remove_request_buffer_from_map (i->first); /* delete it * * Deleting the ringbuffer destroys all RequestObjects diff --git a/libs/pbd/pbd/event_loop.h b/libs/pbd/pbd/event_loop.h index 35e7aeb9e7..5f24bbeeba 100644 --- a/libs/pbd/pbd/event_loop.h +++ b/libs/pbd/pbd/event_loop.h @@ -99,15 +99,13 @@ public: struct ThreadBufferMapping { pthread_t emitting_thread; - std::string target_thread_name; - void* request_buffer; + size_t num_requests; }; static std::vector get_request_buffers_for_target_thread (const std::string&); - static void register_request_buffer_factory (const std::string& target_thread_name, void* (*factory) (uint32_t)); static void pre_register (const std::string& emitting_thread_name, uint32_t num_requests); - static void remove_request_buffer_from_map (void* ptr); + static void remove_request_buffer_from_map (pthread_t); std::list trash; @@ -115,9 +113,9 @@ private: static Glib::Threads::Private thread_event_loop; std::string _name; - typedef std::map ThreadRequestBufferList; + typedef std::vector ThreadRequestBufferList; static ThreadRequestBufferList thread_buffer_requests; - static Glib::Threads::RWLock thread_buffer_requests_lock; + static Glib::Threads::Mutex thread_buffer_requests_lock; struct RequestBufferSupplier {