From b82d2856b4da6549517c6924090f0d892a36d6b5 Mon Sep 17 00:00:00 2001 From: Kevin Harwell <kharwell@digium.com> Date: Tue, 22 Jan 2019 12:07:04 -0600 Subject: [PATCH] res_pjsip_registrar: mitigate blocked threads on reliable transport shutdown When a reliable transport is shutdown it's possible for the pjsip registrar resource shutdown handler to get called multiple times. If this happens and one of the threads is taking "too long" (slow database call for instance) then the others get blocked waiting to delete. Since it only takes one to delete the contact then the other threads should be able to continue on if one of the threads is currently "deleting". This patch makes it so now when a thread enters the shutdown handler it checks to see if a thread is currently already "deleting". If so, then the thread does not attempt to get the lock, and instead continues on thus avoiding the blockage. ASTERISK-28213 #close Change-Id: I7563ca596312b1dff4f3ab41483e89fe2862328a --- res/res_pjsip_registrar.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c index efd2bd9397..83fbdf8168 100644 --- a/res/res_pjsip_registrar.c +++ b/res/res_pjsip_registrar.c @@ -318,6 +318,8 @@ struct contact_transport_monitor { * \note Stored after aor_name in space reserved when struct allocated. */ char *contact_name; + /*! Indicates that the monitor is in the process of removing a contact */ + int removing; /*! AOR name the contact is associated */ char aor_name[0]; }; @@ -344,6 +346,20 @@ static int register_contact_transport_remove_cb(void *data) } ao2_lock(aor); + + /* + * We're now locked so check again to make sure some other thread is not + * currently removing the contact, or already has. + */ + if (monitor->removing) { + ao2_unlock(aor); + ao2_ref(aor, -1); + ao2_ref(monitor, -1); + return 0; + } + + monitor->removing = 1; + contact = ast_sip_location_retrieve_contact(monitor->contact_name); if (contact) { ast_sip_location_delete_contact(contact); @@ -379,6 +395,19 @@ static void register_contact_transport_shutdown_cb(void *data) { struct contact_transport_monitor *monitor = data; + /* + * It's possible for this shutdown handler to get called multiple times for the + * same monitor from different threads. Only one of the calls needs to do the + * actual removing of the contact, so if one is currently removing then any + * subsequent calls can skip. + * + * We'll call it non locked here, but check again once locked just in case the + * flag was updated (see register_contact_transport_remove_cb). + */ + if (monitor->removing) { + return; + } + /* * Push off to a default serializer. This is in case sorcery * does database accesses for contacts. Database accesses may -- GitLab