pjsip: Match lifetime of INVITE session to our session.

In some circumstances it was possible for an INVITE
session to be destroyed while we were still using it.
This occurred due to the reference on the INVITE session
being released internally as a result of its state
changing to DISCONNECTED.

This change adds a reference to the INVITE session
which is released when our own session is destroyed,
ensuring that the INVITE session remains valid for
the lifetime of our session.

ASTERISK-29022

Change-Id: I300c6d9005ff0e6efbe1132daefc7e47ca6228c9

diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index 93889fc11135d6c4ec1e1ef8ea935ab3f8407fac..b6944dbc7ee699e30d5f56598cf4d45218bab0b9 100644 (file)
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -689,9 +689,6 @@ static int answer(void *data)
                ast_log(LOG_ERROR, "Session already DISCONNECTED [reason=%d (%s)]\n",
                        session->inv_session->cause,
                        pjsip_get_status_text(session->inv_session->cause)->ptr);
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-               pjsip_inv_dec_ref(session->inv_session);
-#endif
                SCOPE_EXIT_RTN_VALUE(0, "Disconnected\n");
        }
 
@@ -708,10 +705,6 @@ static int answer(void *data)
                ast_sip_session_send_response(session, packet);
        }
 
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       pjsip_inv_dec_ref(session->inv_session);
-#endif
-
        if (status != PJ_SUCCESS) {
                char err[PJ_ERR_MSG_SIZE];
 
@@ -744,14 +737,6 @@ static int chan_pjsip_answer(struct ast_channel *ast)
        ast_setstate(ast, AST_STATE_UP);
        session = ao2_bump(channel->session);
 
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       if (pjsip_inv_add_ref(session->inv_session) != PJ_SUCCESS) {
-               ast_log(LOG_ERROR, "Couldn't increase the session reference counter\n");
-               ao2_ref(session, -1);
-               SCOPE_EXIT_RTN_VALUE(-1, "Couldn't increase the session reference counter\n");
-       }
-#endif
-
        /* the answer task needs to be pushed synchronously otherwise a race condition
           can occur between this thread and bridging (specifically when native bridging
           attempts to do direct media) */
@@ -763,9 +748,6 @@ static int chan_pjsip_answer(struct ast_channel *ast)
                if (res == -1) {
                        ast_log(LOG_ERROR,"Cannot answer '%s': Unable to push answer task to the threadpool.\n",
                                ast_channel_name(session->channel));
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-                       pjsip_inv_dec_ref(session->inv_session);
-#endif
                }
                ao2_ref(session, -1);
                ast_channel_lock(ast);
@@ -1358,9 +1340,6 @@ static int indicate(void *data)
                ast_sip_session_send_response(session, packet);
        }
 
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       pjsip_inv_dec_ref(session->inv_session);
-#endif
        ao2_ref(ind_data, -1);
 
        return 0;
@@ -1392,31 +1371,20 @@ static int transmit_info_with_vidupdate(void *data)
                ast_log(LOG_ERROR, "Session already DISCONNECTED [reason=%d (%s)]\n",
                        session->inv_session->cause,
                        pjsip_get_status_text(session->inv_session->cause)->ptr);
-               goto failure;
+               return -1;
        }
 
        if (ast_sip_create_request("INFO", session->inv_session->dlg, session->endpoint, NULL, NULL, &tdata)) {
                ast_log(LOG_ERROR, "Could not create text video update INFO request\n");
-               goto failure;
+               return -1;
        }
        if (ast_sip_add_body(tdata, &body)) {
                ast_log(LOG_ERROR, "Could not add body to text video update INFO request\n");
-               goto failure;
+               return -1;
        }
        ast_sip_session_send_request(session, tdata);
 
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       pjsip_inv_dec_ref(session->inv_session);
-#endif
-
        return 0;
-
-failure:
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       pjsip_inv_dec_ref(session->inv_session);
-#endif
-       return -1;
-
 }
 
 /*!
@@ -1464,9 +1432,6 @@ static int update_connected_line_information(void *data)
                ast_log(LOG_ERROR, "Session already DISCONNECTED [reason=%d (%s)]\n",
                        session->inv_session->cause,
                        pjsip_get_status_text(session->inv_session->cause)->ptr);
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-               pjsip_inv_dec_ref(session->inv_session);
-#endif
                ao2_ref(session, -1);
                return -1;
        }
@@ -1507,10 +1472,6 @@ static int update_connected_line_information(void *data)
                }
        }
 
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       pjsip_inv_dec_ref(session->inv_session);
-#endif
-
        ao2_ref(session, -1);
        return 0;
 }
@@ -1742,18 +1703,9 @@ static int chan_pjsip_indicate(struct ast_channel *ast, int condition, const voi
                                        res = ast_rtp_instance_write(media->rtp, &fr);
                                } else {
                                        ao2_ref(channel->session, +1);
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-                                       if (pjsip_inv_add_ref(channel->session->inv_session) != PJ_SUCCESS) {
-                                               ast_log(LOG_ERROR, "Can't increase the session reference counter\n");
+                                       if (ast_sip_push_task(channel->session->serializer, transmit_info_with_vidupdate, channel->session)) {
                                                ao2_cleanup(channel->session);
-                                       } else {
-#endif
-                                               if (ast_sip_push_task(channel->session->serializer, transmit_info_with_vidupdate, channel->session)) {
-                                                       ao2_cleanup(channel->session);
-                                               }
-#ifdef HAVE_PJSIP_INV_SESSION_REF
                                        }
-#endif
                                }
                                ast_test_suite_event_notify("AST_CONTROL_VIDUPDATE", "Result: Success");
                        } else {
@@ -1767,17 +1719,7 @@ static int chan_pjsip_indicate(struct ast_channel *ast, int condition, const voi
                break;
        case AST_CONTROL_CONNECTED_LINE:
                ao2_ref(channel->session, +1);
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-               if (pjsip_inv_add_ref(channel->session->inv_session) != PJ_SUCCESS) {
-                       ao2_cleanup(channel->session);
-                       SCOPE_EXIT_LOG_RTN_VALUE(-1, LOG_ERROR, "%s: Couldn't increase the session reference counter\n",
-                               ast_channel_name(ast));
-               }
-#endif
                if (ast_sip_push_task(channel->session->serializer, update_connected_line_information, channel->session)) {
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-                       pjsip_inv_dec_ref(channel->session->inv_session);
-#endif
                        ao2_cleanup(channel->session);
                }
                break;
@@ -1884,18 +1826,10 @@ static int chan_pjsip_indicate(struct ast_channel *ast, int condition, const voi
                if (!ind_data) {
                        SCOPE_EXIT_LOG_RTN_VALUE(-1, LOG_ERROR, "%s: Couldn't alloc indicate data\n", ast_channel_name(ast));
                }
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-               if (pjsip_inv_add_ref(ind_data->session->inv_session) != PJ_SUCCESS) {
-                       ao2_cleanup(ind_data);
-                       SCOPE_EXIT_LOG_RTN_VALUE(-1, LOG_ERROR, "%s: Couldn't increase the session reference counter\n", ast_channel_name(ast));
-               }
-#endif
+
                if (ast_sip_push_task(channel->session->serializer, indicate, ind_data)) {
                        ast_log(LOG_ERROR, "%s: Cannot send response code %d to endpoint %s. Could not queue task properly\n",
                                        ast_channel_name(ast), response_code, ast_sorcery_object_get_id(channel->session->endpoint));
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-                       pjsip_inv_dec_ref(ind_data->session->inv_session);
-#endif
                        ao2_cleanup(ind_data);
                        res = -1;
                }
@@ -2166,10 +2100,6 @@ static int transfer(void *data)
                }
        }
 
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       pjsip_inv_dec_ref(trnf_data->session->inv_session);
-#endif
-
        ao2_ref(trnf_data, -1);
        ao2_cleanup(endpoint);
        ao2_cleanup(contact);
@@ -2186,19 +2116,8 @@ static int chan_pjsip_transfer(struct ast_channel *chan, const char *target)
                return -1;
        }
 
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       if (pjsip_inv_add_ref(trnf_data->session->inv_session) != PJ_SUCCESS) {
-               ast_log(LOG_ERROR, "Can't increase the session reference counter\n");
-               ao2_cleanup(trnf_data);
-               return -1;
-       }
-#endif
-
        if (ast_sip_push_task(channel->session->serializer, transfer, trnf_data)) {
                ast_log(LOG_WARNING, "Error requesting transfer\n");
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-               pjsip_inv_dec_ref(trnf_data->session->inv_session);
-#endif
                ao2_cleanup(trnf_data);
                return -1;
        }
@@ -2293,12 +2212,12 @@ static int transmit_info_dtmf(void *data)
                ast_log(LOG_ERROR, "Session already DISCONNECTED [reason=%d (%s)]\n",
                        session->inv_session->cause,
                        pjsip_get_status_text(session->inv_session->cause)->ptr);
-               goto failure;
+               return -1;
        }
 
        if (!(body_text = ast_str_create(32))) {
                ast_log(LOG_ERROR, "Could not allocate buffer for INFO DTMF.\n");
-               goto failure;
+               return -1;
        }
        ast_str_set(&body_text, 0, "Signal=%c\r\nDuration=%u\r\n", dtmf_data->digit, dtmf_data->duration);
 
@@ -2306,27 +2225,16 @@ static int transmit_info_dtmf(void *data)
 
        if (ast_sip_create_request("INFO", session->inv_session->dlg, session->endpoint, NULL, NULL, &tdata)) {
                ast_log(LOG_ERROR, "Could not create DTMF INFO request\n");
-               goto failure;
+               return -1;
        }
        if (ast_sip_add_body(tdata, &body)) {
                ast_log(LOG_ERROR, "Could not add body to DTMF INFO request\n");
                pjsip_tx_data_dec_ref(tdata);
-               goto failure;
+               return -1;
        }
        ast_sip_session_send_request(session, tdata);
 
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       pjsip_inv_dec_ref(session->inv_session);
-#endif
-
        return 0;
-
-failure:
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       pjsip_inv_dec_ref(session->inv_session);
-#endif
-       return -1;
-
 }
 
 /*! \brief Function called by core to stop a DTMF digit */
@@ -2367,19 +2275,8 @@ static int chan_pjsip_digit_end(struct ast_channel *ast, char digit, unsigned in
                        return -1;
                }
 
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-               if (pjsip_inv_add_ref(dtmf_data->session->inv_session) != PJ_SUCCESS) {
-                       ast_log(LOG_ERROR, "Can't increase the session reference counter\n");
-                       ao2_cleanup(dtmf_data);
-                       return -1;
-               }
-#endif
-
                if (ast_sip_push_task(channel->session->serializer, transmit_info_dtmf, dtmf_data)) {
                        ast_log(LOG_WARNING, "Error sending DTMF via INFO.\n");
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-                       pjsip_inv_dec_ref(dtmf_data->session->inv_session);
-#endif
                        ao2_cleanup(dtmf_data);
                        return -1;
                }
@@ -2888,10 +2785,6 @@ static int sendtext(void *obj)
                ast_sip_send_request(tdata, data->session->inv_session->dlg, data->session->endpoint, NULL, NULL);
        }
 
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       pjsip_inv_dec_ref(data->session->inv_session);
-#endif
-
        ao2_cleanup(data);
 
        return 0;
@@ -2913,18 +2806,7 @@ static int chan_pjsip_sendtext_data(struct ast_channel *ast, struct ast_msg_data
                return -1;
        }
 
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       if (pjsip_inv_add_ref(data->session->inv_session) != PJ_SUCCESS) {
-               ast_log(LOG_ERROR, "Can't increase the session reference counter\n");
-               ao2_ref(data, -1);
-               return -1;
-       }
-#endif
-
        if (ast_sip_push_task(channel->session->serializer, sendtext, data)) {
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-               pjsip_inv_dec_ref(data->session->inv_session);
-#endif
                ao2_ref(data, -1);
                return -1;
        }

diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 693d432fbcdc9bf8b6fa07b6e8bf21a7286c6ff9..696835414e92a6f56dac91ecb636b2d15159c938 100644 (file)
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -2997,7 +2997,16 @@ static void session_destructor(void *obj)
        ast_dsp_free(session->dsp);
 
        if (session->inv_session) {
-               pjsip_dlg_dec_session(session->inv_session->dlg, &session_module);
+               struct pjsip_dialog *dlg = session->inv_session->dlg;
+
+               /* The INVITE session uses the dialog pool for memory, so we need to
+                * decrement its reference first before that of the dialog.
+                */
+
+#ifdef HAVE_PJSIP_INV_SESSION_REF
+               pjsip_inv_dec_ref(session->inv_session);
+#endif
+               pjsip_dlg_dec_session(dlg, &session_module);
        }
 
        ast_test_suite_event_notify("SESSION_DESTROYED", "Endpoint: %s", endpoint_name);
@@ -3105,6 +3114,24 @@ struct ast_sip_session *ast_sip_session_alloc(struct ast_sip_endpoint *endpoint,
        }
        ast_sip_dialog_set_serializer(inv_session->dlg, session->serializer);
        ast_sip_dialog_set_endpoint(inv_session->dlg, endpoint);
+
+       /* When a PJSIP INVITE session is created it is created with a reference
+        * count of 1, with that reference being managed by the underlying state
+        * of the INVITE session itself. When the INVITE session transitions to
+        * a DISCONNECTED state that reference is released. This means we can not
+        * rely on that reference to ensure the INVITE session remains for the
+        * lifetime of our session. To ensure it does we add our own reference
+        * and release it when our own session goes away, ensuring that the INVITE
+        * session remains for the lifetime of session.
+        */
+
+#ifdef HAVE_PJSIP_INV_SESSION_REF
+       if (pjsip_inv_add_ref(inv_session) != PJ_SUCCESS) {
+               ast_log(LOG_ERROR, "Can't increase the session reference counter\n");
+               return NULL;
+       }
+#endif
+
        pjsip_dlg_inc_session(inv_session->dlg, &session_module);
        inv_session->mod_data[session_module.id] = ao2_bump(session);
        session->contact = ao2_bump(contact);
@@ -3956,9 +3983,6 @@ static int new_invite(struct new_invite *invite)
                        ast_sip_session_get_name(invite->session),
                        invite->session->inv_session->cause,
                        pjsip_get_status_text(invite->session->inv_session->cause)->ptr);
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-               pjsip_inv_dec_ref(invite->session->inv_session);
-#endif
                SCOPE_EXIT_RTN_VALUE(-1);
        }
 
@@ -4076,9 +4100,6 @@ static int new_invite(struct new_invite *invite)
        handle_incoming_request(invite->session, invite->rdata);
 
 end:
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       pjsip_inv_dec_ref(invite->session->inv_session);
-#endif
        SCOPE_EXIT_RTN_VALUE(0, "%s\n", ast_sip_session_get_name(invite->session));
 }
 
@@ -4124,19 +4145,6 @@ static void handle_new_invite_request(pjsip_rx_data *rdata)
         * process handling has successfully completed.
         */
 
-
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-       if (pjsip_inv_add_ref(inv_session) != PJ_SUCCESS) {
-               ast_log(LOG_ERROR, "Can't increase the session reference counter\n");
-               /* Dialog's lock and a reference are removed in new_invite_initial_answer */
-               if (!new_invite_initial_answer(inv_session, rdata, 500, 500, PJ_FALSE)) {
-                       /* Terminate the session if it wasn't done in the answer */
-                       pjsip_inv_terminate(inv_session, 500, PJ_FALSE);
-               }
-               SCOPE_EXIT_RTN("Couldn't add invite session reference\n");
-       }
-#endif
-
        session = ast_sip_session_alloc(endpoint, NULL, inv_session, rdata);
        if (!session) {
                /* Dialog's lock and reference are removed in new_invite_initial_answer */
@@ -4144,9 +4152,6 @@ static void handle_new_invite_request(pjsip_rx_data *rdata)
                        /* Terminate the session if it wasn't done in the answer */
                        pjsip_inv_terminate(inv_session, 500, PJ_FALSE);
                }
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-               pjsip_inv_dec_ref(inv_session);
-#endif
                SCOPE_EXIT_RTN("Couldn't create session\n");
        }
        session->call_direction = AST_SIP_SESSION_INCOMING_CALL;