res_pjsip_pubsub: subscription cleanup changes

There are two main parts of the change associated with this
commit. These are driven by the change in call order of
pubsub_on_rx_refresh and pubsub_on_evsub_state by pjproject
when an in-dialog SUBSCRIBE is received.

First, the previous behavior was for pjproject to call
pubsub_on_rx_refresh before calling pubsub_on_evsub_state
when an in-dialog SUBSCRIBE was received that changes the
subscription state.

If that change was a termination due to a re-SUBSCRIBE with
an expires of 0, we used to use the call to pubsub_on_rx_refresh
to set the substate of the evsub to TERMINATE_PENDING before
pjproject could call pubsub_on_evsub_state.

This substate let pubsub_on_evsub_state know that the
subscription TERMINATED event could be ignored as there was
still a subsequent NOTIFY that needed to be generated and
another call to pubsub_on_evsub_state to come with it.

That NOTIFY was sent via serialized_pubsub_on_refresh_timeout
which would see the TERMINATE_PENDING state and transition it
to TERMINATE_IN_PROGRESS before triggering another call to
pubsub_on_evsub_state (which now would clean up the evsub.)

The new pjproject behavior is to call pubsub_on_evsub_state
before pubsub_on_rx_refresh. This means we no longer can set
the state to TERMINATE_PENDING to tell pubsub_on_evsub_state
that it can ignore the first TERMINATED event.

To handle this, we now look directly at the event type,
method type and the expires value to determine whether we
want to ignore the event or use it to trigger the evsub
cleanup.

Second, pjproject now expects the NOTIFY to actually be sent
during pubsub_on_rx_refresh and avoids the protocol violation
inherent in sending a NOTIFY before the SUBSCRIBE is
acknowledged by caching the sent NOTIFY then sending it
after responding to the SUBSCRIBE.

This requires we send the NOTIFY using the non-serialized
pubsub_on_refresh_timeout directly and let pjproject handle
the protocol violation.

ASTERISK-30469

Change-Id: I05c1d91a44fe28244ae93faa4a2268a3332b5fd7

res/res_pjsip_pubsub.c

@@ -3773,15 +3773,38 @@ static void set_state_terminated(struct ast_sip_subscription *sub)
 /*!
  * \brief Callback sequence for subscription terminate:
  *
+ * * Please note that the descriptions below represent pjproject behavior on versions
+ *   >= 2.13.
  * * Client initiated:
  *     pjproject receives SUBSCRIBE on the subscription's serializer thread
+ *         calls pubsub_evsub_set_state with state = TERMINATED
+ *             pubsub_on_evsub_state checks the event and finds it is due to a received
+ *             SUBSCRIBE with an expires of 0 and so does nothing.
  *         calls pubsub_on_rx_refresh with dialog locked
  *             pubsub_on_rx_refresh sets TERMINATE_PENDING
+ *             calls pubsub_on_refresh_timeout to push final NOTIFY to pjproject
+ *                 checks state == TERMINATE_PENDING
+ *                 sets TERMINATE_IN_PROGRESS
+ *                 calls send_notify (2)
+ *                 send_notify ultimately calls pjsip_evsub_send_request
+ *                 pjsip_evsub_send_request calls evsub's set_state
+ *                     set_state calls pubsub_evsub_set_state
+ *                         pubsub_on_evsub_state checks state == TERMINATE_IN_PROGRESS
+ *                         removes the subscriptions
+ *                         cleans up references to evsub
+ *                         sets state = TERMINATED
+ *             pubsub_on_refresh_timeout unlocks dialog
+ *             returns to pjproject
+ *         pjproject unlocks dialog
+ *
+ * * Subscription timer expires:
+ *     pjproject timer expires
+ *         locks dialog
+ *         calls pubsub_on_server_timeout
+ *             pubsub_on_server_timeout checks state == NORMAL
+ *             sets TERMINATE_PENDING
  *             pushes serialized_pubsub_on_refresh_timeout
  *             returns to pjproject
- *         pjproject calls pubsub_on_evsub_state
- *             pubsub_evsub_set_state checks state == TERMINATE_IN_PROGRESS (no)
- *             ignore and return
  *         pjproject unlocks dialog
  *     serialized_pubsub_on_refresh_timeout starts (1)
  *       locks dialog
@@ -3792,23 +3815,12 @@ static void set_state_terminated(struct ast_sip_subscription *sub)
  *               pjsip_evsub_send_request calls evsub's set_state
  *                   set_state calls pubsub_evsub_set_state
  *                       pubsub_on_evsub_state checks state == TERMINATE_IN_PROGRESS
+ *                       checks that the event is not due to un-SUBSCRIBE
  *                       removes the subscriptions
  *                       cleans up references to evsub
  *                       sets state = TERMINATED
  *       serialized_pubsub_on_refresh_timeout unlocks dialog
  *
- * * Subscription timer expires:
- *     pjproject timer expires
- *         locks dialog
- *         calls pubsub_on_server_timeout
- *             pubsub_on_server_timeout checks state == NORMAL
- *             sets TERMINATE_PENDING
- *             pushes serialized_pubsub_on_refresh_timeout
- *             returns to pjproject
- *         pjproject unlocks dialog
- *     serialized_pubsub_on_refresh_timeout starts
- *         See (1) Above
- *
  * * Transmission failure sending NOTIFY or error response from client
  *     pjproject transaction timer expires or non OK response
  *         pjproject locks dialog
@@ -3839,20 +3851,14 @@ static void set_state_terminated(struct ast_sip_subscription *sub)
  */
 
 
-/* The code in this function was previously in pubsub_on_evsub_state. As of
+/* The code in this function was previously in pubsub_on_evsub_state. */
- * pjsip 2.13 pubsub_on_evsub_state is called before pubsub_on_rx_refresh, so
- * if we clean the sub tree in pubsub_on_evsub_state it won't be available in
- * pubsub_on_rx_refresh. This means we won't be able to build or send the
- * corresponding NOTIFY (which also causes pjsip to assert.)
- * If HAVE_PJSIP_EVSUB_PENDING_NOTIFY is set based on configuration, this will
- * be called from pubsub_on_rx_refresh. If not set, the result is the legacy
- * behavior of calling this from pubsub_on_evsub_state.
- */
 static void clean_sub_tree(pjsip_evsub *evsub){
 
 	struct sip_subscription_tree *sub_tree;
 	sub_tree = pjsip_evsub_get_mod_data(evsub, pubsub_module.id);
 
+	ast_debug(3, "Cleaning subscription %p\n", evsub);
+
 	if (sub_tree->expiration_task) {
 		char task_name[256];
 
@@ -3915,12 +3921,22 @@ static void pubsub_on_evsub_state(pjsip_evsub *evsub, pjsip_event *event)
 		return;
 	}
 
-#ifndef HAVE_PJSIP_EVSUB_PENDING_NOTIFY
+#ifdef HAVE_PJSIP_EVSUB_PENDING_NOTIFY
-	/* for pjproject <2.13, this cleanup occurs here.  For >=2.13, pubsub_on_evsub_state
+	/* This check looks for re-subscribes with an expires of 0. If we receive one of those,
-	   is called before pubsub_on_rx_refresh and so must be cleaned there.*/
+	   we don't want to clean the evsub because we still need it to send the final NOTIFY.
-	clean_sub_tree(evsub);
+	   This was previously handled by pubsub_on_rx_refresh setting:
+	   'sub_tree->state = SIP_SUB_TREE_TERMINATE_PENDING' */
+	if (event->body.tsx_state.type == PJSIP_EVENT_RX_MSG &&
+	    !pjsip_method_cmp(&event->body.tsx_state.tsx->method, &pjsip_subscribe_method) &&
+	    pjsip_evsub_get_expires(evsub) == 0) {
+
+		ast_debug(3, "Subscription ending, do nothing.\n");
+		return;
+	}
 #endif
-
+	/* If we made it this far, we want to clean the sub tree. For pjproject <2.13, the sub_tree
+	   state check makes sure the evsub is not cleaned at the wrong time */
+	clean_sub_tree(evsub);
 }
 
 static int pubsub_on_refresh_timeout(void *userdata)
@@ -4036,8 +4052,7 @@ static int destroy_subscriptions_task(void *obj)
  * This includes both SUBSCRIBE requests that actually refresh the subscription
  * as well as SUBSCRIBE requests that end the subscription.
  *
- * In either case we push serialized_pubsub_on_refresh_timeout to send an
+ * In either case we push an appropriate NOTIFY via pubsub_on_refresh_timeout.
- * appropriate NOTIFY request.
  */
 static void pubsub_on_rx_refresh(pjsip_evsub *evsub, pjsip_rx_data *rdata,
 		int *p_st_code, pj_str_t **p_st_text, pjsip_hdr *res_hdr, pjsip_msg_body **p_body)
@@ -4135,8 +4150,8 @@ static void pubsub_on_rx_refresh(pjsip_evsub *evsub, pjsip_rx_data *rdata,
 	/* As of pjsip 2.13, the NOTIFY has to be sent within this function as pjproject now
 	   requires it.  Previously this would have caused an early NOTIFY to go out before the
 	   SUBSCRIBE's 200 OK. The previous solution was to push the NOTIFY, but now pjproject
-	   looks for the NOTIFY on send and delays it until after it auto-replies.
+	   looks for the NOTIFY to be sent from this function and caches it to send after it
-	   If the NOTIFY is not there when it looks to send, pjproject will assert. */
+	   auto-replies to the SUBSCRIBE. */
 	pubsub_on_refresh_timeout(sub_tree);
 #else
 	if (ast_sip_push_task(sub_tree->serializer, serialized_pubsub_on_refresh_timeout, ao2_bump(sub_tree))) {
@@ -4150,18 +4165,6 @@ static void pubsub_on_rx_refresh(pjsip_evsub *evsub, pjsip_rx_data *rdata,
 	if (sub_tree->is_list) {
 		pj_list_insert_before(res_hdr, create_require_eventlist(rdata->tp_info.pool));
 	}
-
-#ifdef HAVE_PJSIP_EVSUB_PENDING_NOTIFY
-	/* for pjproject <2.13, this cleanup occurs in pubsub_on_evsub_state.  For >=2.13,
-	   pubsub_on_rx_refresh is called after pubsub_on_evsub_state and so the tree must be
-	   cleaned here. */
-	if( pjsip_evsub_get_state(evsub) == PJSIP_EVSUB_STATE_TERMINATED &&
-		sub_tree->state == SIP_SUB_TREE_TERMINATE_PENDING ) {
-			clean_sub_tree(evsub);
-	}
-#endif
-
-
 }
 
 static void pubsub_on_rx_notify(pjsip_evsub *evsub, pjsip_rx_data *rdata, int *p_st_code,