[Asterisk-code-review] res pjsip session: Handle race condition at shutdown with ti... (asterisk[13])

[Joshua Colp]

Joshua Colp has uploaded a new change for review.

  https://gerrit.asterisk.org/3048

Change subject: res_pjsip_session: Handle race condition at shutdown with timer.
......................................................................

res_pjsip_session: Handle race condition at shutdown with timer.

When shutting down res_pjsip_session will get unloaded before res_pjsip.
The act of unloading unregisters all the PJSIP services and sets
their module IDs to -1. In some cases it is possible for a timer to
occur after this happens which calls into res_pjsip_session. The
res_pjsip_session module can then try to get the session from the
INVITE session using the module ID. Since the module ID is now -1
this fails.

This change stores a copy of the module ID and uses it for the timer
callback scenario. If the module ID is -1 the callback immediately
returns but if the module ID is valid then it continues as normal.

This works as even though the module may be unregistered the ID when
used on the INVITE session will still be valid.

ASTERISK-26127 #close

Change-Id: I88df72525c4e9ef9f19c13aedddd3ac4a335c573
---
M res/res_pjsip_session.c
1 file changed, 14 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/48/3048/1

diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 18bb6ab..1ee1fae 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -2668,13 +2668,25 @@
 		 * by the session serializer.
 		 */
 		if (inv->state == PJSIP_INV_STATE_DISCONNECTED) {
+			int id = session_module.id;
+
+			/*
+			 * A race condition exists at shutdown where the res_pjsip_session can be
+			 * unloaded but this callback may still get called afterwards. In this case
+			 * the id may end up being -1 which is useless to us. To work around this
+			 * we store the current value and check/use it.
+			 */
+			if (id < 0) {
+				return;
+			}
+
 			/*
 			 * We are locking because ast_sip_dialog_get_session() needs
 			 * the dialog locked to get the session by other threads.
 			 */
 			pjsip_dlg_inc_lock(inv->dlg);
-			session = inv->mod_data[session_module.id];
-			inv->mod_data[session_module.id] = NULL;
+			session = inv->mod_data[id];
+			inv->mod_data[id] = NULL;
 			pjsip_dlg_dec_lock(inv->dlg);
 
 			/*

-- 
To view, visit https://gerrit.asterisk.org/3048
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I88df72525c4e9ef9f19c13aedddd3ac4a335c573
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Joshua Colp <jcolp at digium.com>