Skip to content

Commit ac0db0a

Browse files
lminierolminiero
authored
Prevent race conditions on socket close in SIP and NoSIP plugins (#2599)
1 parent 9533345 commit ac0db0a

File tree

2 files changed

+67
-13
lines changed

2 files changed

+67
-13
lines changed

‎plugins/janus_nosip.c

+28-5
Original file line numberDiff line numberDiff line change
@@ -1238,7 +1238,9 @@ static void janus_nosip_hangup_media_internal(janus_plugin_session *handle) {
12381238
}
12391239
/* Do cleanup if media thread has not been created */
12401240
if(!session->media.ready && !session->relayer_thread) {
1241+
janus_mutex_lock(&session->mutex);
12411242
janus_nosip_media_cleanup(session);
1243+
janus_mutex_unlock(&session->mutex);
12421244
}
12431245
/* Get rid of the recorders, if available */
12441246
janus_mutex_lock(&session->rec_mutex);
@@ -1438,13 +1440,16 @@ static void *janus_nosip_handler(void *data) {
14381440
JANUS_LOG(LOG_VERB, "Going to negotiate video...\n");
14391441
session->media.has_video = 1; /* FIXME Maybe we need a better way to signal this */
14401442
}
1443+
janus_mutex_lock(&session->mutex);
14411444
if(janus_nosip_allocate_local_ports(session, sdp_update) < 0) {
1445+
janus_mutex_unlock(&session->mutex);
14421446
JANUS_LOG(LOG_ERR, "Could not allocate RTP/RTCP ports\n");
14431447
janus_sdp_destroy(parsed_sdp);
14441448
error_code = JANUS_NOSIP_ERROR_IO_ERROR;
14451449
g_snprintf(error_cause, 512, "Could not allocate RTP/RTCP ports");
14461450
goto error;
14471451
}
1452+
janus_mutex_unlock(&session->mutex);
14481453

14491454
char *sdp = janus_nosip_sdp_manipulate(session, parsed_sdp, FALSE);
14501455
if(sdp == NULL) {
@@ -2223,6 +2228,14 @@ static void *janus_nosip_relay_thread(void *data) {
22232228
int pipe_fd = session->media.pipefd[0];
22242229
char buffer[1500];
22252230
memset(buffer, 0, 1500);
2231+
if(pipe_fd == -1) {
2232+
/* If the pipe file descriptor doesn't exist, it means we're done already,
2233+
* and/or we may never be notified about sessions being closed, so give up */
2234+
JANUS_LOG(LOG_WARN, "[NoSIP-%p] Leaving thread, no pipe file descriptor...\n", session);
2235+
janus_refcount_decrease(&session->ref);
2236+
g_thread_unref(g_thread_self());
2237+
return NULL;
2238+
}
22262239
/* Loop */
22272240
int num = 0;
22282241
gboolean goon = TRUE;
@@ -2311,12 +2324,16 @@ static void *janus_nosip_relay_thread(void *data) {
23112324
fds[num].revents = 0;
23122325
num++;
23132326
}
2314-
if(pipe_fd != -1) {
2315-
fds[num].fd = pipe_fd;
2316-
fds[num].events = POLLIN;
2317-
fds[num].revents = 0;
2318-
num++;
2327+
/* Finally, let's add the pipe */
2328+
pipe_fd = session->media.pipefd[0];
2329+
if(pipe_fd == -1) {
2330+
/* Pipe was closed? Means the call is over */
2331+
break;
23192332
}
2333+
fds[num].fd = pipe_fd;
2334+
fds[num].events = POLLIN;
2335+
fds[num].revents = 0;
2336+
num++;
23202337
/* Wait for some data */
23212338
resfd = poll(fds, num, 1000);
23222339
if(resfd < 0) {
@@ -2351,13 +2368,17 @@ static void *janus_nosip_relay_thread(void *data) {
23512368
if(fds[i].fd == session->media.audio_rtcp_fd) {
23522369
JANUS_LOG(LOG_WARN, "[NoSIP-%p] Got a '%s' on the audio RTCP socket, closing it\n",
23532370
session, g_strerror(error));
2371+
janus_mutex_lock(&session->mutex);
23542372
close(session->media.audio_rtcp_fd);
23552373
session->media.audio_rtcp_fd = -1;
2374+
janus_mutex_unlock(&session->mutex);
23562375
} else if(fds[i].fd == session->media.video_rtcp_fd) {
23572376
JANUS_LOG(LOG_WARN, "[NoSIP-%p] Got a '%s' on the video RTCP socket, closing it\n",
23582377
session, g_strerror(error));
2378+
janus_mutex_lock(&session->mutex);
23592379
close(session->media.video_rtcp_fd);
23602380
session->media.video_rtcp_fd = -1;
2381+
janus_mutex_unlock(&session->mutex);
23612382
}
23622383
}
23632384
/* FIXME Should we be more tolerant of ICMP errors on RTP sockets as well? */
@@ -2484,7 +2505,9 @@ static void *janus_nosip_relay_thread(void *data) {
24842505
}
24852506
}
24862507
/* Cleanup the media session */
2508+
janus_mutex_lock(&session->mutex);
24872509
janus_nosip_media_cleanup(session);
2510+
janus_mutex_unlock(&session->mutex);
24882511
/* Done */
24892512
JANUS_LOG(LOG_INFO, "Leaving NoSIP relay thread\n");
24902513
session->relayer_thread = NULL;

‎plugins/janus_sip.c

+39-8
Original file line numberDiff line numberDiff line change
@@ -2544,7 +2544,9 @@ static void janus_sip_hangup_media_internal(janus_plugin_session *handle) {
25442544
session->media.simulcast_ssrc = 0;
25452545
/* Do cleanup if media thread has not been created */
25462546
if(!session->media.ready && !session->relayer_thread) {
2547+
janus_mutex_lock(&session->mutex);
25472548
janus_sip_media_cleanup(session);
2549+
janus_mutex_unlock(&session->mutex);
25482550
}
25492551
/* Get rid of the recorders, if available */
25502552
janus_mutex_lock(&session->rec_mutex);
@@ -3396,13 +3398,16 @@ static void *janus_sip_handler(void *data) {
33963398
JANUS_LOG(LOG_VERB, "Going to negotiate video...\n");
33973399
session->media.has_video = TRUE; /* FIXME Maybe we need a better way to signal this */
33983400
}
3401+
janus_mutex_lock(&session->mutex);
33993402
if(janus_sip_allocate_local_ports(session, FALSE) < 0) {
3403+
janus_mutex_unlock(&session->mutex);
34003404
JANUS_LOG(LOG_ERR, "Could not allocate RTP/RTCP ports\n");
34013405
janus_sdp_destroy(parsed_sdp);
34023406
error_code = JANUS_SIP_ERROR_IO_ERROR;
34033407
g_snprintf(error_cause, 512, "Could not allocate RTP/RTCP ports");
34043408
goto error;
34053409
}
3410+
janus_mutex_unlock(&session->mutex);
34063411
char *sdp = janus_sip_sdp_manipulate(session, parsed_sdp, FALSE);
34073412
if(sdp == NULL) {
34083413
JANUS_LOG(LOG_ERR, "Error manipulating SDP\n");
@@ -3694,13 +3699,16 @@ static void *janus_sip_handler(void *data) {
36943699
JANUS_LOG(LOG_VERB, "Going to negotiate video...\n");
36953700
session->media.has_video = TRUE; /* FIXME Maybe we need a better way to signal this */
36963701
}
3702+
janus_mutex_lock(&session->mutex);
36973703
if(janus_sip_allocate_local_ports(session, FALSE) < 0) {
3704+
janus_mutex_unlock(&session->mutex);
36983705
JANUS_LOG(LOG_ERR, "Could not allocate RTP/RTCP ports\n");
36993706
janus_sdp_destroy(parsed_sdp);
37003707
error_code = JANUS_SIP_ERROR_IO_ERROR;
37013708
g_snprintf(error_cause, 512, "Could not allocate RTP/RTCP ports");
37023709
goto error;
37033710
}
3711+
janus_mutex_unlock(&session->mutex);
37043712
char *sdp = janus_sip_sdp_manipulate(session, parsed_sdp, TRUE);
37053713
if(sdp == NULL) {
37063714
JANUS_LOG(LOG_ERR, "Could not allocate RTP/RTCP ports\n");
@@ -3883,13 +3891,16 @@ static void *janus_sip_handler(void *data) {
38833891
session->media.has_srtp_local_video = session->media.has_srtp_remote_video;
38843892
}
38853893
if(audio_added || video_added) {
3894+
janus_mutex_lock(&session->mutex);
38863895
if(janus_sip_allocate_local_ports(session, TRUE) < 0) {
3896+
janus_mutex_unlock(&session->mutex);
38873897
JANUS_LOG(LOG_ERR, "Could not allocate RTP/RTCP ports\n");
38883898
janus_sdp_destroy(parsed_sdp);
38893899
error_code = JANUS_SIP_ERROR_IO_ERROR;
38903900
g_snprintf(error_cause, 512, "Could not allocate RTP/RTCP ports");
38913901
goto error;
38923902
}
3903+
janus_mutex_unlock(&session->mutex);
38933904
if(!offer)
38943905
session->media.updated = TRUE;
38953906
}
@@ -6252,8 +6263,10 @@ static int janus_sip_allocate_local_ports(janus_sip_session *session, gboolean u
62526263
session->media.local_video_rtcp_port = rtcp_port;
62536264
}
62546265
}
6255-
/* We need this to quickly interrupt the poll when it's time to update a session or wrap up */
6256-
pipe(session->media.pipefd);
6266+
if(!update) {
6267+
/* We need this to quickly interrupt the poll when it's time to update a session or wrap up */
6268+
pipe(session->media.pipefd);
6269+
}
62576270
return 0;
62586271
}
62596272

@@ -6353,7 +6366,7 @@ static void *janus_sip_relay_thread(void *data) {
63536366
JANUS_LOG(LOG_VERB, "Starting relay thread (%s <--> %s)\n", session->account.username, session->callee);
63546367

63556368
if(!session->callee) {
6356-
JANUS_LOG(LOG_VERB, "[SIP-%s] Leaving thread, no callee...\n", session->account.username);
6369+
JANUS_LOG(LOG_WARN, "[SIP-%s] Leaving thread, no callee...\n", session->account.username);
63576370
janus_refcount_decrease(&session->ref);
63586371
g_thread_unref(g_thread_self());
63596372
return NULL;
@@ -6366,6 +6379,14 @@ static void *janus_sip_relay_thread(void *data) {
63666379
int pipe_fd = session->media.pipefd[0];
63676380
char buffer[1500];
63686381
memset(buffer, 0, 1500);
6382+
if(pipe_fd == -1) {
6383+
/* If the pipe file descriptor doesn't exist, it means we're done already,
6384+
* and/or we may never be notified about sessions being closed, so give up */
6385+
JANUS_LOG(LOG_WARN, "[SIP-%s] Leaving thread, no pipe file descriptor...\n", session->account.username);
6386+
janus_refcount_decrease(&session->ref);
6387+
g_thread_unref(g_thread_self());
6388+
return NULL;
6389+
}
63696390
/* Loop */
63706391
int num = 0;
63716392
gboolean goon = TRUE;
@@ -6462,12 +6483,16 @@ static void *janus_sip_relay_thread(void *data) {
64626483
fds[num].revents = 0;
64636484
num++;
64646485
}
6465-
if(pipe_fd != -1) {
6466-
fds[num].fd = pipe_fd;
6467-
fds[num].events = POLLIN;
6468-
fds[num].revents = 0;
6469-
num++;
6486+
/* Finally, let's add the pipe */
6487+
pipe_fd = session->media.pipefd[0];
6488+
if(pipe_fd == -1) {
6489+
/* Pipe was closed? Means the call is over */
6490+
break;
64706491
}
6492+
fds[num].fd = pipe_fd;
6493+
fds[num].events = POLLIN;
6494+
fds[num].revents = 0;
6495+
num++;
64716496
/* Wait for some data */
64726497
resfd = poll(fds, num, 1000);
64736498
if(resfd < 0) {
@@ -6504,14 +6529,18 @@ static void *janus_sip_relay_thread(void *data) {
65046529
if(fds[i].fd == session->media.audio_rtcp_fd) {
65056530
JANUS_LOG(LOG_WARN, "[SIP-%s] Got a '%s' on the audio RTCP socket, closing it\n",
65066531
session->account.username, g_strerror(error));
6532+
janus_mutex_lock(&session->mutex);
65076533
close(session->media.audio_rtcp_fd);
65086534
session->media.audio_rtcp_fd = -1;
6535+
janus_mutex_unlock(&session->mutex);
65096536
continue;
65106537
} else if(fds[i].fd == session->media.video_rtcp_fd) {
65116538
JANUS_LOG(LOG_WARN, "[SIP-%s] Got a '%s' on the video RTCP socket, closing it\n",
65126539
session->account.username, g_strerror(error));
6540+
janus_mutex_lock(&session->mutex);
65136541
close(session->media.video_rtcp_fd);
65146542
session->media.video_rtcp_fd = -1;
6543+
janus_mutex_unlock(&session->mutex);
65156544
continue;
65166545
}
65176546
}
@@ -6687,7 +6716,9 @@ static void *janus_sip_relay_thread(void *data) {
66876716
}
66886717
}
66896718
/* Cleanup the media session */
6719+
janus_mutex_lock(&session->mutex);
66906720
janus_sip_media_cleanup(session);
6721+
janus_mutex_unlock(&session->mutex);
66916722
/* Done */
66926723
JANUS_LOG(LOG_VERB, "Leaving SIP relay thread\n");
66936724
session->relayer_thread = NULL;

0 commit comments

Comments
 (0)