From 4283325bfe9ef9488af76510ee886bc87aa7152f Mon Sep 17 00:00:00 2001
From: George Yang <g.yang@genexis.eu>
Date: Thu, 23 May 2024 08:58:25 +0000
Subject: [PATCH] Fix a deadlock issue triggered by R1
Deadlock happens between threads which respectively call chan_voicemngr_getRtpStats() or
chan_voicemngr_indicate() to lock sub->parent and ast_channel objects.
---
src/channels/chan_voicemngr.c | 32 ++++++++++----------------------
1 file changed, 10 insertions(+), 22 deletions(-)
diff --git a/src/channels/chan_voicemngr.c b/src/channels/chan_voicemngr.c
index 73efc1d..3e14c9b 100644
--- a/src/channels/chan_voicemngr.c
+++ b/src/channels/chan_voicemngr.c
@@ -133,7 +133,7 @@ static struct ast_channel *chan_voicemngr_new(struct chan_voicemngr_subchannel *
struct ast_format_cap *format);
static int handle_dialtone_timeout(const void *data);
static int handle_congestion_timeout(const void *data);
-static int endpt_get_rtp_stats(int line);
+static int endpt_get_rtp_stats(struct chan_voicemngr_subchannel *sub);
static int is_call_waiting_enabled(const char *sip_account);
static int has_call_in_sip_client(const char *sip_account);
static struct ast_format *map_rtpname_to_format(char* name);
@@ -1154,8 +1154,9 @@ static int chan_voicemngr_getRtpStats(struct ast_channel *ast)
}
if (sub->parent) {
+ chan_voicemngr_get_sip_client_id(sub);
pvt_lock(sub->parent, "chan_voicemngr_getRtpStats");
- if (endpt_get_rtp_stats(sub->parent->line_id)) {
+ if (endpt_get_rtp_stats(sub)) {
ast_log(LOG_WARNING, "Unable to get RTP statistics\n");
}
pvt_unlock(sub->parent);
@@ -4948,12 +4949,14 @@ static void ubus_call_answer_rtp_stats(struct ubus_request *req, int type, struc
return;
}
- sub = chan_voicemngr_get_active_subchannel(p);
+ sub = (struct chan_voicemngr_subchannel *)req->priv;
if (!sub) {
- ast_log(LOG_ERROR, "No active subchannel to write rtp stats!\n");
+ ast_log(LOG_ERROR, "No subchannel found!\n");
return;
}
+ sip_client_id = sub->sip_client_id;
+
if (tb[RTP_STATS_LOCAL_BURST_DENSITY])
sub->rtp_stats.localBurstDensity = blobmsg_get_u16(tb[RTP_STATS_LOCAL_BURST_DENSITY]);
if (tb[RTP_STATS_REMOTE_BURST_DENSITY])
@@ -4990,7 +4993,6 @@ static void ubus_call_answer_rtp_stats(struct ubus_request *req, int type, struc
sub->rtp_stats.discarded = blobmsg_get_u32(tb[RTP_STATS_DISCARDED]);
if (tb[RTP_STATS_LOST]) {
sub->rtp_stats.lost = blobmsg_get_u32(tb[RTP_STATS_LOST]);
- sip_client_id = chan_voicemngr_get_sip_client_id(sub);
if (sip_client_id >= 0 && sip_client_id < MAX_SIP_CLIENTS) {
line_stats[sip_client_id].pktslost += sub->rtp_stats.lost;
}
@@ -5011,14 +5013,12 @@ static void ubus_call_answer_rtp_stats(struct ubus_request *req, int type, struc
sub->rtp_stats.maxJitter = blobmsg_get_u32(tb[RTP_STATS_MAX_JITTER]);
if (tb[RTP_STATS_OVERRUNS]) {
overruns = blobmsg_get_u16(tb[RTP_STATS_OVERRUNS]);
- sip_client_id = chan_voicemngr_get_sip_client_id(sub);
if (sip_client_id >= 0 && sip_client_id < MAX_SIP_CLIENTS) {
line_stats[sip_client_id].total_overruns += overruns;
}
}
if (tb[RTP_STATS_UNDERRUNS]) {
underruns = blobmsg_get_u16(tb[RTP_STATS_UNDERRUNS]);
- sip_client_id = chan_voicemngr_get_sip_client_id(sub);
if (sip_client_id >= 0 && sip_client_id < MAX_SIP_CLIENTS) {
line_stats[sip_client_id].total_underruns += underruns;
}
@@ -5050,29 +5050,17 @@ static void ubus_call_answer_rtp_stats(struct ubus_request *req, int type, struc
sub->rtp_stats.localAvgRoundTripDelay, sub->rtp_stats.remoteAvgRoundTripDelay, overruns, underruns);
}
-static int endpt_get_rtp_stats(int line) {
+static int endpt_get_rtp_stats(struct chan_voicemngr_subchannel *sub) {
+ int line = sub->parent->line_id;
struct ubus_context *local_ctx;
struct blob_buf bb;
int ret;
- struct chan_voicemngr_pvt *p = NULL;
- struct chan_voicemngr_subchannel *sub = NULL;
/*
* Reset rtp_stats first because ubus_call_answer_rtp_stats() will not be called if "ubus call endpt rtp_stats" fails,
* e.g. an unanswered incoming call on which the connection is not created. In this case, all RTP statistics counters
* shall be zeros.
*/
- p = chan_voicemngr_get_pvt_from_lineid(iflist, line);
- if (!p) {
- ast_log(LOG_ERROR, "No pvt with the line %d found!\n", line);
- return -1;
- }
-
- sub = chan_voicemngr_get_active_subchannel(p);
- if (!sub) {
- ast_log(LOG_ERROR, "No active subchannel to get rtp stats!\n");
- return -1;
- }
memset(&sub->rtp_stats, 0, sizeof(sub->rtp_stats));
if (!endpt_id) {
@@ -5094,7 +5082,7 @@ static int endpt_get_rtp_stats(int line) {
blobmsg_add_u8(&bb, "reset", 1); // always reset RTP stats after get them
ast_log(LOG_DEBUG, "thread %d: ubus call endpt rtp_stats \"{'line':%d,'reset':true}\"", ast_get_tid(), line);
- ret = ubus_invoke(local_ctx, endpt_id, "rtp_stats", bb.head, ubus_call_answer_rtp_stats, NULL, 500);
+ ret = ubus_invoke(local_ctx, endpt_id, "rtp_stats", bb.head, ubus_call_answer_rtp_stats, sub, 500);
blob_buf_free(&bb);
ast_ubus_free_context(local_ctx);
--
GitLab