[Asterisk-code-review] res pjsip registrar: mitigate blocked threads on reliable tr... (asterisk[master])

[Joshua C. Colp]

Joshua C. Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/10909 )

Change subject: res_pjsip_registrar: mitigate blocked threads on reliable transport shutdown
......................................................................

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
---
M res/res_pjsip_registrar.c
1 file changed, 29 insertions(+), 0 deletions(-)

Approvals:
  Sean Bright: Looks good to me, but someone else must approve
  Joshua C. Colp: Looks good to me, approved; Approved for Submit



diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c
index efd2bd9..83fbdf8 100644
--- a/res/res_pjsip_registrar.c
+++ b/res/res_pjsip_registrar.c
@@ -318,6 +318,8 @@
 	 * \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 @@
 	}
 
 	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);
@@ -380,6 +396,19 @@
 	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
 	 * not be on this machine.  We don't want to tie up the pjsip

-- 
To view, visit https://gerrit.asterisk.org/10909
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7563ca596312b1dff4f3ab41483e89fe2862328a
Gerrit-Change-Number: 10909
Gerrit-PatchSet: 3
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Friendly Automation (1000185)
Gerrit-Reviewer: Joshua C. Colp <jcolp at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190124/a31e676a/attachment-0001.html>