[Asterisk-code-review] res_pjsip_pubsub: fix Batched Notifications stop working (asterisk[19])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/17996 )

Change subject: res_pjsip_pubsub: fix Batched Notifications stop working
......................................................................

res_pjsip_pubsub: fix Batched Notifications stop working

If Subscription refresh occurred between when the batched notification
was scheduled and the serialized notification was to be sent,
then new schedule notification task would never be added.

There are 2 threads:

thread #1. ast_sip_subscription_notify is called,
if notification_batch_interval then call schedule_notification.
1.1. The schedule_notification checks notify_sched_id > -1
not true, then
send_scheduled_notify = 1
notify_sched_id =
  ast_sched_add(sched, sub_tree->notification_batch_interval, sched_cb....
1.2. The sched_cb pushes task serialized_send_notify to serializer
and returns 0 which means no reschedule.
1.3. The serialized_send_notify checks send_scheduled_notify if it's false
the just returns. BUT notify_sched_id is still set, so no more ast_sched_add.

thread #2. pubsub_on_rx_refresh is called
2.1 it pushes serialized_pubsub_on_refresh_timeout to serializer
2.2. The serialized_pubsub_on_refresh_timeout calls pubsub_on_refresh_timeout
which calls send_notify
2.3. The send_notify set send_scheduled_notify = 0;

The serialized_send_notify should always unset notify_sched_id.

ASTERISK-29904 #close

Change-Id: Ifc50c00b213c396509e10326a1ed89d8cf8c7875
---
M res/res_pjsip_pubsub.c
1 file changed, 2 insertions(+), 1 deletion(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved
  Friendly Automation: Approved for Submit



diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index 60506db..dae3fef 100644
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -2492,6 +2492,8 @@
 
 	pjsip_dlg_inc_lock(dlg);
 
+	sub_tree->notify_sched_id = -1;
+
 	/* It's possible that between when the notification was scheduled
 	 * and now a new SUBSCRIBE arrived requiring full state to be
 	 * sent out in an immediate NOTIFY. It's also possible that we're
@@ -2517,7 +2519,6 @@
 			? "SUBSCRIPTION_TERMINATED" : "SUBSCRIPTION_STATE_CHANGED",
 		"Resource: %s", sub_tree->root->resource);
 
-	sub_tree->notify_sched_id = -1;
 	pjsip_dlg_dec_lock(dlg);
 	ao2_cleanup(sub_tree);
 	return 0;

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

Gerrit-Project: asterisk
Gerrit-Branch: 19
Gerrit-Change-Id: Ifc50c00b213c396509e10326a1ed89d8cf8c7875
Gerrit-Change-Number: 17996
Gerrit-PatchSet: 2
Gerrit-Owner: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220223/5c1c312a/attachment.html>