[Asterisk-code-review] Further fixes to improper usage of scheduler (asterisk[certified/11.6])

[Matt Jordan]

Matt Jordan has submitted this change and it was merged.

Change subject: Further fixes to improper usage of scheduler
......................................................................


Further fixes to improper usage of scheduler

When ASTERISK-25449 was closed, a number of scheduler issues mentioned in
the comments were missed. These have since beed raised in ASTERISK-25476
and elsewhere.

This patch attempts to collect all of the scheduler issues discovered so
far and address them sensibly.

ASTERISK-25476 #close

Change-Id: I87a77d581e2e0d91d33b4b2fbff80f64a566d05b
(cherry picked from commit e74110188d7e4c959d6c3ddbe40635a639b33a14)
---
M channels/chan_iax2.c
M channels/chan_sip.c
M res/res_rtp_asterisk.c
3 files changed, 16 insertions(+), 10 deletions(-)

Approvals:
  Matt Jordan: Looks good to me, approved
  Joshua Colp: Looks good to me, but someone else must approve; Verified



diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index df945fe..a492bdd 100644
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -7061,7 +7061,7 @@
 
 	p = find_peer(a->argv[2], 1);
 	if (p) {
-		if (p->expire > 0) {
+		if (p->expire > -1) {
 			struct iax2_peer *peer;
 
 			peer = ao2_find(peers, a->argv[2], OBJ_KEY);
@@ -7094,7 +7094,7 @@
 		struct ao2_iterator i = ao2_iterator_init(peers, 0);
 		while ((p = ao2_iterator_next(&i))) {
 			if (!strncasecmp(p->name, word, wordlen) && 
-				++which > state && p->expire > 0) {
+				++which > state && p->expire > -1) {
 				res = ast_strdup(p->name);
 				peer_unref(p);
 				break;
diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 53070cd..c47baf7 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -20312,7 +20312,7 @@
 		return CLI_SHOWUSAGE;
 	
 	if ((peer = sip_find_peer(a->argv[2], NULL, load_realtime, FINDPEERS, TRUE, 0))) {
-		if (peer->expire > 0) {
+		if (peer->expire > -1) {
 			AST_SCHED_DEL_UNREF(sched, peer->expire,
 				sip_unref_peer(peer, "remove register expire ref"));
 			expire_register(sip_ref_peer(peer, "ref for expire_register"));
@@ -20895,7 +20895,7 @@
        while ((peer = ao2_t_iterator_next(&i, "iterate thru peers table"))) {
 	       if (!strncasecmp(word, peer->name, wordlen) &&
 		   (!flags2 || ast_test_flag(&peer->flags[1], flags2)) &&
-		   ++which > state && peer->expire > 0)
+		   ++which > state && peer->expire > -1)
 		       result = ast_strdup(peer->name);
 	       if (result) {
 		       sip_unref_peer(peer, "toss iterator peer ptr before break");
@@ -30215,13 +30215,11 @@
 /*! \brief Set peer defaults before configuring specific configurations */
 static void set_peer_defaults(struct sip_peer *peer)
 {
-	if (peer->expire == 0) {
+	if (peer->expire < 0) {
 		/* Don't reset expire or port time during reload
 		   if we have an active registration
 		*/
-		peer->expire = -1;
-		peer->pokeexpire = -1;
-		peer->keepalivesend = -1;
+		peer_sched_cleanup(peer);
 		set_socket_transport(&peer->socket, SIP_TRANSPORT_UDP);
 	}
 	peer->type = SIP_TYPE_PEER;
@@ -30303,6 +30301,10 @@
 	}
 
 	ast_atomic_fetchadd_int(&apeerobjs, 1);
+	peer->expire = -1;
+	peer->pokeexpire = -1;
+	peer->keepalivesend = -1;
+
 	set_peer_defaults(peer);
 
 	ast_copy_string(peer->name, name, sizeof(peer->name));
@@ -30422,6 +30424,10 @@
 			ast_debug(3, "-REALTIME- peer built. Name: %s. Peer objects: %d\n", name, rpeerobjs);
 		} else
 			ast_atomic_fetchadd_int(&speerobjs, 1);
+
+		peer->expire = -1;
+		peer->pokeexpire = -1;
+		peer->keepalivesend = -1;
 	}
 
 	/* Note that our peer HAS had its reference count increased */
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 7b7f34a..f10dee8 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -4275,7 +4275,7 @@
 			return;
 		} else {
 			if (rtp->rtcp) {
-				if (rtp->rtcp->schedid > 0) {
+				if (rtp->rtcp->schedid > -1) {
 					if (!ast_sched_del(rtp->sched, rtp->rtcp->schedid)) {
 						/* Successfully cancelled scheduler entry. */
 						ao2_ref(instance, -1);
@@ -4501,7 +4501,7 @@
 	ast_mutex_unlock(&rtp->dtls_timer_lock);
 #endif
 
-	if (rtp->rtcp && rtp->rtcp->schedid > 0) {
+	if (rtp->rtcp && rtp->rtcp->schedid > -1) {
 		if (!ast_sched_del(rtp->sched, rtp->rtcp->schedid)) {
 			/* successfully cancelled scheduler entry. */
 			ao2_ref(instance, -1);

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I87a77d581e2e0d91d33b4b2fbff80f64a566d05b
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/11.6
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>