res_pjsip_session: Add cleanup to ast_sip_session_terminate
authorGeorge Joseph <gjoseph@digium.com>
Thu, 27 Apr 2017 13:02:12 +0000 (07:02 -0600)
committerGeorge Joseph <gjoseph@digium.com>
Thu, 27 Apr 2017 15:43:32 +0000 (10:43 -0500)
If you use ast_request to create a PJSIP channel but then hang it
up without causing a transaction to be sent, the session will
never be destroyed.  This is due ot the fact that it's pjproject
that triggers the session cleanup when the transaction ends.
app_chanisavail was doing this to get more granular channel state
and it's also possible for this to happen via ARI.

* ast_sip_session_terminate was modified to explicitly call the
  cleanup tasks and unreference session if the invite state is NULL
  AND invite_tsx is NULL (meaning we never sent a transaction).

* chan_pjsip/hangup was modified to bump session before it calls
  ast_sip_session_terminate to insure that session stays valid
  while it does its own cleanup.

* Added test events to session_destructor for a future testsuite
  test.

ASTERISK-26908 #close
Reported-by: Richard Mudgett

Change-Id: I52daf6f757184e5544c261f64f6fe9602c4680a9

channels/chan_pjsip.c
include/asterisk/res_pjsip_session.h
res/res_pjsip_session.c

index a928164..5bf339e 100644 (file)
@@ -2052,11 +2052,16 @@ static int hangup(void *data)
        struct ast_sip_session *session = channel->session;
        int cause = h_data->cause;
 
-       ast_sip_session_terminate(session, cause);
+       /*
+        * It's possible that session_terminate might cause the session to be destroyed
+        * immediately so we need to keep a reference to it so we can NULL session->channel
+        * afterwards.
+        */
+       ast_sip_session_terminate(ao2_bump(session), cause);
        clear_session_and_channel(session, ast, pvt);
+       ao2_cleanup(session);
        ao2_cleanup(channel);
        ao2_cleanup(h_data);
-
        return 0;
 }
 
index d4d3f70..10e55f1 100644 (file)
@@ -459,6 +459,10 @@ struct ast_sip_session *ast_sip_session_create_outgoing(struct ast_sip_endpoint
  *
  * \param session The session to terminate
  * \param response The response code to use for termination if possible
+ *
+ * \warning Calling this function MAY cause the last session reference to be
+ * released and the session destructor to be called.  If you need to do something
+ * with session after this call, be sure to bump the ref count before calling terminate.
  */
 void ast_sip_session_terminate(struct ast_sip_session *session, int response);
 
index 3034652..2613a74 100644 (file)
@@ -1302,9 +1302,19 @@ static void session_destructor(void *obj)
        struct ast_sip_session *session = obj;
        struct ast_sip_session_supplement *supplement;
        struct ast_sip_session_delayed_request *delay;
+       const char *endpoint_name = session->endpoint ?
+               ast_sorcery_object_get_id(session->endpoint) : "<none>";
 
-       ast_debug(3, "Destroying SIP session with endpoint %s\n",
-               session->endpoint ? ast_sorcery_object_get_id(session->endpoint) : "<none>");
+       ast_debug(3, "Destroying SIP session with endpoint %s\n", endpoint_name);
+
+       ast_test_suite_event_notify("SESSION_DESTROYING",
+               "Endpoint: %s\r\n"
+               "AOR: %s\r\n"
+               "Contact: %s"
+               , endpoint_name
+               , session->aor ? ast_sorcery_object_get_id(session->aor) : "<none>"
+               , session->contact ? ast_sorcery_object_get_id(session->contact) : "<none>"
+               );
 
        while ((supplement = AST_LIST_REMOVE_HEAD(&session->supplements, next))) {
                if (supplement->session_destroy) {
@@ -1333,6 +1343,8 @@ static void session_destructor(void *obj)
        if (session->inv_session) {
                pjsip_dlg_dec_session(session->inv_session->dlg, &session_module);
        }
+
+       ast_test_suite_event_notify("SESSION_DESTROYED", "Endpoint: %s", endpoint_name);
 }
 
 static int add_supplements(struct ast_sip_session *session)
@@ -1793,6 +1805,9 @@ struct ast_sip_session *ast_sip_session_create_outgoing(struct ast_sip_endpoint
        return ret_session;
 }
 
+static int session_end(void *vsession);
+static int session_end_completion(void *vsession);
+
 void ast_sip_session_terminate(struct ast_sip_session *session, int response)
 {
        pj_status_t status;
@@ -1809,7 +1824,25 @@ void ast_sip_session_terminate(struct ast_sip_session *session, int response)
 
        switch (session->inv_session->state) {
        case PJSIP_INV_STATE_NULL:
-               pjsip_inv_terminate(session->inv_session, response, PJ_TRUE);
+               if (!session->inv_session->invite_tsx) {
+                       /*
+                        * Normally, it's pjproject's transaction cleanup that ultimately causes the
+                        * final session reference to be released but if both STATE and invite_tsx are NULL,
+                        * we never created a transaction in the first place.  In this case, we need to
+                        * do the cleanup ourselves.
+                        */
+                       /* Transfer the inv_session session reference to the session_end_task */
+                       session->inv_session->mod_data[session_module.id] = NULL;
+                       pjsip_inv_terminate(session->inv_session, response, PJ_TRUE);
+                       session_end(session);
+                       /*
+                        * session_end_completion will cleanup the final session reference unless
+                        * ast_sip_session_terminate's caller is holding one.
+                        */
+                       session_end_completion(session);
+               } else {
+                       pjsip_inv_terminate(session->inv_session, response, PJ_TRUE);
+               }
                break;
        case PJSIP_INV_STATE_CONFIRMED:
                if (session->inv_session->invite_tsx) {