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

[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 07583c288828a496cd7730b55112128fea31eaef)
---
M channels/chan_iax2.c
M channels/chan_sip.c
M res/res_rtp_asterisk.c
3 files changed, 17 insertions(+), 11 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Matt Jordan: Looks good to me, approved
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index 8128a6d..7d962e2 100644
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -7148,7 +7148,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);
@@ -7180,8 +7180,8 @@
 	if (pos == 2) {
 		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) {
+			if (!strncasecmp(p->name, word, wordlen) && 
+				++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 76803a4..5f38e4d 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -20720,7 +20720,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"));
@@ -21299,7 +21299,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");
@@ -30150,13 +30150,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, AST_TRANSPORT_UDP);
 	}
 	peer->type = SIP_TYPE_PEER;
@@ -30241,6 +30239,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));
@@ -30358,6 +30360,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 678926d..e2fa99a 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -4696,7 +4696,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);
@@ -4911,7 +4911,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/1615
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I87a77d581e2e0d91d33b4b2fbff80f64a566d05b
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.1
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>