From 6b5891a78f31d894e9036d8dcd42564f49f30366 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Wed, 14 Dec 2016 18:46:01 +0100 Subject: [PATCH] The threading anecdotes - Episode 7 When do_request() destroys the receiver object, the receiver will free the invalidation record. So the IR needs to be removed from the list before executing the request. Invalid read of size 8 at: AbstractUI::handle_ui_requests() (abstract_ui.cc:242) by: BaseUI::request_handler(Glib::IOCondition) (base_ui.cc:141) by: sigc::bound_mem_functor1::operator()(Glib::IOCondition const&) const (mem_fun.h:2066) by: sigc::adaptor_functor >::deduce_result_type::type sigc::adaptor_functor >::operator()(Glib::IOCondition const&) const (adaptor_trait.h:89) by: sigc::internal::slot_call1, bool, Glib::IOCondition>::call_it(sigc::internal::slot_rep*, Glib::IOCondition const&) (slot.h:148) by: sigc::slot1::operator()(Glib::IOCondition const&) const (slot.h:643) by: cross_thread_channel_call_receive_slot(_GIOChannel*, GIOCondition, void*) (crossthread.cc:49) by: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5000.2) by: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5000.2) by: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5000.2) by: gtk_main (in /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0.2400.31) by: Gtkmm2ext::UI::run(Receiver&) (gtk_ui.cc:286) by main (main.cc:408) Addrd1b8 is 24 bytes inside a block of size 48 free'd at: operator delete(void*) (vg_replace_malloc.c:576) by: PBD::EventLoop::invalidate_request(void*) (event_loop.cc:98) by: sigc::internal::trackable_callback_list::~trackable_callback_list() (in /usr/lib/x86_64-linux-gnu/libsigc-2.0.so.0.0.0) by: sigc::trackable::notify_callbacks() (in /usr/lib/x86_64-linux-gnu/libsigc-2.0.so.0.0.0) by: ProcessorEntry::LuaPluginDisplay::~LuaPluginDisplay() (processor_box.cc:1757) by: ProcessorEntry::LuaPluginDisplay::~LuaPluginDisplay() (processor_box.cc:1760) by: ProcessorEntry::~ProcessorEntry() (processor_box.cc:251) --- libs/pbd/pbd/abstract_ui.cc | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/libs/pbd/pbd/abstract_ui.cc b/libs/pbd/pbd/abstract_ui.cc index 7fdb213ad0..19a0ba9c47 100644 --- a/libs/pbd/pbd/abstract_ui.cc +++ b/libs/pbd/pbd/abstract_ui.cc @@ -219,8 +219,27 @@ AbstractUI::handle_ui_requests () break; } else { if (vec.buf[0]->valid) { + /* We first need to remove the event from the list. + * If the event results in object destruction, PBD::EventLoop::invalidate_request + * will delete the invalidation record (aka buf[0]), so we cannot use it after calling do_request + */ + DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1: remove request from its invalidation list\n", event_loop_name())); + if (vec.buf[0]->invalidation) { + DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1: removing invalidation record for that request\n", event_loop_name())); + if (vec.buf[0]->invalidation->event_loop && vec.buf[0]->invalidation->event_loop != this) { + vec.buf[0]->invalidation->event_loop->slot_invalidation_mutex().lock (); + } + vec.buf[0]->invalidation->requests.remove (vec.buf[0]); + if (vec.buf[0]->invalidation->event_loop && vec.buf[0]->invalidation->event_loop != this) { + vec.buf[0]->invalidation->event_loop->slot_invalidation_mutex().unlock (); + } + } else { + DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1: no invalidation record for that request\n", event_loop_name())); + } + DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1: valid request, unlocking before calling\n", event_loop_name())); rbml.release (); + DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1: valid request, calling ::do_request()\n", event_loop_name())); do_request (vec.buf[0]); @@ -237,18 +256,6 @@ AbstractUI::handle_ui_requests () } rbml.acquire (); - if (vec.buf[0]->invalidation && !(*i).second->dead) { - DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1: removing invalidation record for that request\n", event_loop_name())); - if (vec.buf[0]->invalidation->event_loop && vec.buf[0]->invalidation->event_loop != this) { - vec.buf[0]->invalidation->event_loop->slot_invalidation_mutex().lock (); - } - vec.buf[0]->invalidation->requests.remove (vec.buf[0]); - if (vec.buf[0]->invalidation->event_loop && vec.buf[0]->invalidation->event_loop != this) { - vec.buf[0]->invalidation->event_loop->slot_invalidation_mutex().unlock (); - } - } else { - DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1: no invalidation record for that request\n", event_loop_name())); - } } else { DEBUG_TRACE (PBD::DEBUG::AbstractUI, "invalid request, ignoring\n"); } @@ -323,7 +330,6 @@ AbstractUI::handle_ui_requests () if (!req->valid) { DEBUG_TRACE (PBD::DEBUG::AbstractUI, string_compose ("%1/%2 handling invalid heap request, type %3, deleting\n", event_loop_name(), pthread_name(), req->type)); delete req; - //rbml.release (); continue; }