Skip to content
Snippets Groups Projects
Unverified Commit 5fd4d2e0 authored by Sébastien Blin's avatar Sébastien Blin
Browse files

Revert "ice: resort the check list when adding prflx candidates"

This reverts commit 5e453368.

Reason for revert: Cause crash due to re-ordering the checklist while some checks are in progress

The resort of is causing inconstitency between the checks. The current uses
the list index as the ID for the STUN request. When the STUN response
(for this request) is received, the ID is used as is to retrieve the
check in the check list.
If the list is re-ordered (when receiving an new incoming check), before
the STUN response is received and processed, the ID of STUN request will
point to a different check on reception of the STUN response.

This a temporary revert until a more robust solution to the ordering
issue is found.

Change-Id: I912a9861118461ff7f6296f61c53531fa2b9384e
parent eb158ad6
No related branches found
No related tags found
No related merge requests found
From 328a6837cc739440ed9c9abe18a43a36d3c4a60d Mon Sep 17 00:00:00 2001
From: sauwming <ming@teluu.com>
Date: Mon, 30 Aug 2021 11:45:52 +0800
Subject: [PATCH 1/2] Resort ICE checklist upon entering nomination stage
(#2806)
---
pjnath/src/pjnath/ice_session.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/pjnath/src/pjnath/ice_session.c b/pjnath/src/pjnath/ice_session.c
index 3fecc3def..ef88807a1 100644
--- a/pjnath/src/pjnath/ice_session.c
+++ b/pjnath/src/pjnath/ice_session.c
@@ -3463,7 +3463,11 @@ static void handle_incoming_check(pj_ice_sess *ice,
LOG4((ice->obj_name, "New triggered check added: %d",
ice->clist.count));
pj_log_push_indent();
+
perform_check(ice, &ice->clist, ice->clist.count++, nominate);
+ /* Re-sort the list because of the newly added pair. */
+ sort_checklist(ice, &ice->clist);
+
pj_log_pop_indent();
} else {
--
2.25.1
pjnath/src/pjnath/ice_session.c | 58 +++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 43 insertions(+), 15 deletions(-)
diff --git a/pjnath/src/pjnath/ice_session.c b/pjnath/src/pjnath/ice_session.c
index ef88807a1..5feb32b21 100644
index 2e35d930e..34e711a93 100644
--- a/pjnath/src/pjnath/ice_session.c
+++ b/pjnath/src/pjnath/ice_session.c
@@ -3382,14 +3382,22 @@ static void handle_incoming_check(pj_ice_sess *ice,
@@ -4038,14 +4038,22 @@ static void handle_incoming_check(pj_ice_sess *ice,
if (c->state == PJ_ICE_SESS_CHECK_STATE_FROZEN ||
c->state == PJ_ICE_SESS_CHECK_STATE_WAITING)
{
......@@ -13,6 +16,7 @@ index ef88807a1..5feb32b21 100644
- pj_log_push_indent();
- perform_check(ice, &ice->clist, i, nominate);
- pj_log_pop_indent();
-
+ /* If we are nominating in regular nomination, don't nominate this
+ * triggered check immediately, just wait for its scheduled check.
+ */
......@@ -22,7 +26,7 @@ index ef88807a1..5feb32b21 100644
+ } else {
+ /* See if we shall nominate this check */
+ pj_bool_t nominate = (c->nominated || ice->is_nominating);
+
+ LOG5((ice->obj_name, "Performing triggered check for "
+ "check %d",i));
+ pj_log_push_indent();
......@@ -32,7 +36,7 @@ index ef88807a1..5feb32b21 100644
} else if (c->state == PJ_ICE_SESS_CHECK_STATE_IN_PROGRESS) {
/* Should retransmit immediately
*/
@@ -3449,7 +3457,7 @@ static void handle_incoming_check(pj_ice_sess *ice,
@@ -4105,7 +4113,7 @@ static void handle_incoming_check(pj_ice_sess *ice,
else if (ice->clist.count < PJ_ICE_MAX_CHECKS) {
pj_ice_sess_check *c = &ice->clist.checks[ice->clist.count];
......@@ -41,7 +45,7 @@ index ef88807a1..5feb32b21 100644
c->lcand = lcand;
c->rcand = rcand;
@@ -3457,19 +3465,38 @@ static void handle_incoming_check(pj_ice_sess *ice,
@@ -4113,14 +4121,34 @@ static void handle_incoming_check(pj_ice_sess *ice,
c->state = PJ_ICE_SESS_CHECK_STATE_WAITING;
c->nominated = rcheck->use_candidate;
c->err_code = PJ_SUCCESS;
......@@ -53,6 +57,8 @@ index ef88807a1..5feb32b21 100644
- LOG4((ice->obj_name, "New triggered check added: %d",
- ice->clist.count));
- pj_log_push_indent();
- perform_check(ice, &ice->clist, ice->clist.count++, nominate);
- pj_log_pop_indent();
+ /* If we are nominating in regular nomination, don't nominate this
+ * newly found pair.
+ */
......@@ -78,12 +84,5 @@ index ef88807a1..5feb32b21 100644
+ pj_log_pop_indent();
+ }
- perform_check(ice, &ice->clist, ice->clist.count++, nominate);
/* Re-sort the list because of the newly added pair. */
sort_checklist(ice, &ice->clist);
- pj_log_pop_indent();
-
} else {
LOG4((ice->obj_name, "Error: unable to perform triggered check: "
"TOO MANY CHECKS IN CHECKLIST!"));
......@@ -22,7 +22,6 @@
"0016-use-larger-Ta-interval.patch",
"0017-auto-register-thread.patch",
"0018-fix-ioqueue-lock-acquire.patch",
"0019-resort-check-list-after-adding-prflx.patch",
"0020-avoid-immediate-nominating-triggered-check.patch",
"0001-win-config.patch",
"0002-win-vs-gnutls.patch",
......
......@@ -64,7 +64,6 @@ pjproject: pjproject-$(PJPROJECT_VERSION).tar.gz .sum-pjproject
$(APPLY) $(SRC)/pjproject/0016-use-larger-Ta-interval.patch
$(APPLY) $(SRC)/pjproject/0017-auto-register-thread.patch
$(APPLY) $(SRC)/pjproject/0018-fix-ioqueue-lock-acquire.patch
$(APPLY) $(SRC)/pjproject/0019-resort-check-list-after-adding-prflx.patch # TODO remove with 2.12 (https://github.com/pjsip/pjproject/pull/2806)
$(APPLY) $(SRC)/pjproject/0020-avoid-immediate-nominating-triggered-check.patch # TODO remove with 2.12 (https://github.com/pjsip/pjproject/issues/2812)
ifdef HAVE_ANDROID
$(APPLY) $(SRC)/pjproject/0001-android.patch
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment