Skip to content

Commit 57e47dc

Browse files
lminierolminiero
authored
Fix to rare deadlock in Streaming plugin (see #2115) (#2141)
1 parent 52efcc4 commit 57e47dc

File tree

1 file changed

+18
-17
lines changed

1 file changed

+18
-17
lines changed

‎plugins/janus_streaming.c

+18-17
Original file line numberDiff line numberDiff line change
@@ -2302,7 +2302,6 @@ json_t *janus_streaming_query_session(janus_plugin_session *handle) {
23022302
janus_mutex_unlock(&sessions_mutex);
23032303
/* What is this user watching, if anything? */
23042304
json_t *info = json_object();
2305-
janus_mutex_lock(&session->mutex);
23062305
janus_streaming_mountpoint *mp = session->mountpoint;
23072306
json_object_set_new(info, "state", json_string(mp ? "watching" : "idle"));
23082307
if(mp) {
@@ -2340,7 +2339,6 @@ json_t *janus_streaming_query_session(janus_plugin_session *handle) {
23402339
}
23412340
janus_refcount_decrease(&mp->ref);
23422341
}
2343-
janus_mutex_unlock(&session->mutex);
23442342
json_object_set_new(info, "hangingup", json_integer(g_atomic_int_get(&session->hangingup)));
23452343
json_object_set_new(info, "destroyed", json_integer(g_atomic_int_get(&session->destroyed)));
23462344
janus_refcount_decrease(&session->ref);
@@ -4404,6 +4402,8 @@ static void janus_streaming_hangup_media_internal(janus_plugin_session *handle)
44044402
session->paused = FALSE;
44054403
janus_mutex_lock(&session->mutex);
44064404
janus_streaming_mountpoint *mp = session->mountpoint;
4405+
session->mountpoint = NULL;
4406+
janus_mutex_unlock(&session->mutex);
44074407
if(mp) {
44084408
janus_mutex_lock(&mp->mutex);
44094409
JANUS_LOG(LOG_VERB, " -- Removing the session from the mountpoint viewers\n");
@@ -4434,8 +4434,6 @@ static void janus_streaming_hangup_media_internal(janus_plugin_session *handle)
44344434
}
44354435
janus_mutex_unlock(&mp->mutex);
44364436
}
4437-
session->mountpoint = NULL;
4438-
janus_mutex_unlock(&session->mutex);
44394437
g_atomic_int_set(&session->hangingup, 0);
44404438
}
44414439

@@ -4542,16 +4540,16 @@ static void *janus_streaming_handler(void *data) {
45424540
janus_mutex_unlock(&mountpoints_mutex);
45434541
goto error;
45444542
}
4545-
janus_mutex_lock(&session->mutex);
45464543
janus_mutex_lock(&mp->mutex);
4544+
janus_mutex_lock(&session->mutex);
45474545
janus_mutex_unlock(&mountpoints_mutex);
45484546
/* Check if this is a new viewer, or if an update is taking place (i.e., ICE restart) */
45494547
if(do_restart) {
45504548
/* User asked for an ICE restart: provide a new offer */
45514549
if(!g_atomic_int_compare_and_exchange(&session->renegotiating, 0, 1)) {
45524550
/* Already triggered a renegotiation, and still waiting for an answer */
4553-
janus_mutex_unlock(&mp->mutex);
45544551
janus_mutex_unlock(&session->mutex);
4552+
janus_mutex_unlock(&mp->mutex);
45554553
JANUS_LOG(LOG_ERR, "Already renegotiating mountpoint %s\n", session->mountpoint->id_str);
45564554
error_code = JANUS_STREAMING_ERROR_INVALID_STATE;
45574555
g_snprintf(error_cause, 512, "Already renegotiating mountpoint %s", session->mountpoint->id_str);
@@ -4566,33 +4564,33 @@ static void *janus_streaming_handler(void *data) {
45664564
if(session->mountpoint != NULL) {
45674565
if(session->mountpoint != mp) {
45684566
/* Already watching something else */
4569-
janus_mutex_unlock(&mp->mutex);
45704567
janus_refcount_decrease(&mp->ref);
45714568
JANUS_LOG(LOG_ERR, "Already watching mountpoint %s\n", session->mountpoint->id_str);
45724569
error_code = JANUS_STREAMING_ERROR_INVALID_STATE;
45734570
g_snprintf(error_cause, 512, "Already watching mountpoint %s", session->mountpoint->id_str);
45744571
janus_mutex_unlock(&session->mutex);
4572+
janus_mutex_unlock(&mp->mutex);
45754573
goto error;
45764574
} else {
45774575
/* Make sure it's not an API error */
45784576
if(!session->started) {
45794577
/* Can't be a renegotiation, PeerConnection isn't up yet */
4580-
janus_mutex_unlock(&mp->mutex);
45814578
JANUS_LOG(LOG_ERR, "Already watching mountpoint %s\n", session->mountpoint->id_str);
45824579
error_code = JANUS_STREAMING_ERROR_INVALID_STATE;
45834580
g_snprintf(error_cause, 512, "Already watching mountpoint %s", session->mountpoint->id_str);
45844581
janus_refcount_decrease(&mp->ref);
45854582
janus_mutex_unlock(&session->mutex);
4583+
janus_mutex_unlock(&mp->mutex);
45864584
goto error;
45874585
}
45884586
if(!g_atomic_int_compare_and_exchange(&session->renegotiating, 0, 1)) {
45894587
/* Already triggered a renegotiation, and still waiting for an answer */
4590-
janus_mutex_unlock(&mp->mutex);
45914588
JANUS_LOG(LOG_ERR, "Already renegotiating mountpoint %s\n", session->mountpoint->id_str);
45924589
error_code = JANUS_STREAMING_ERROR_INVALID_STATE;
45934590
g_snprintf(error_cause, 512, "Already renegotiating mountpoint %s", session->mountpoint->id_str);
45944591
janus_refcount_decrease(&mp->ref);
45954592
janus_mutex_unlock(&session->mutex);
4593+
janus_mutex_unlock(&mp->mutex);
45964594
goto error;
45974595
}
45984596
/* Simple renegotiation, remove the extra uneeded reference */
@@ -4605,9 +4603,9 @@ static void *janus_streaming_handler(void *data) {
46054603
/* New viewer: we send an offer ourselves */
46064604
JANUS_LOG(LOG_VERB, "Request to watch mountpoint/stream %s\n", id_value_str);
46074605
if(session->mountpoint != NULL || g_list_find(mp->viewers, session) != NULL) {
4606+
janus_mutex_unlock(&session->mutex);
46084607
janus_mutex_unlock(&mp->mutex);
46094608
janus_refcount_decrease(&mp->ref);
4610-
janus_mutex_unlock(&session->mutex);
46114609
JANUS_LOG(LOG_ERR, "Already watching a stream...\n");
46124610
error_code = JANUS_STREAMING_ERROR_UNKNOWN_ERROR;
46134611
g_snprintf(error_cause, 512, "Already watching a stream");
@@ -4631,9 +4629,9 @@ static void *janus_streaming_handler(void *data) {
46314629
(!mp->video || !session->video) &&
46324630
(!mp->data || !session->data)) {
46334631
session->mountpoint = NULL;
4632+
janus_mutex_unlock(&session->mutex);
46344633
janus_mutex_unlock(&mp->mutex);
46354634
janus_refcount_decrease(&mp->ref);
4636-
janus_mutex_unlock(&session->mutex);
46374635
JANUS_LOG(LOG_ERR, "Can't offer an SDP with no audio, video or data for this mountpoint\n");
46384636
error_code = JANUS_STREAMING_ERROR_INVALID_REQUEST;
46394637
g_snprintf(error_cause, 512, "Can't offer an SDP with no audio, video or data for this mountpoint");
@@ -4648,11 +4646,11 @@ static void *janus_streaming_handler(void *data) {
46484646
g_thread_try_new(tname, &janus_streaming_ondemand_thread, session, &error);
46494647
if(error != NULL) {
46504648
session->mountpoint = NULL;
4651-
janus_mutex_unlock(&mp->mutex);
4649+
janus_mutex_unlock(&session->mutex);
46524650
janus_refcount_decrease(&session->ref); /* This is for the failed thread */
4651+
janus_mutex_unlock(&mp->mutex);
46534652
janus_refcount_decrease(&mp->ref); /* This is for the failed thread */
46544653
janus_refcount_decrease(&mp->ref);
4655-
janus_mutex_unlock(&session->mutex);
46564654
JANUS_LOG(LOG_ERR, "Got error %d (%s) trying to launch the on-demand thread...\n",
46574655
error->code, error->message ? error->message : "??");
46584656
error_code = JANUS_STREAMING_ERROR_UNKNOWN_ERROR;
@@ -4668,9 +4666,9 @@ static void *janus_streaming_handler(void *data) {
46684666
JANUS_STREAMING_ERROR_MISSING_ELEMENT, JANUS_STREAMING_ERROR_INVALID_ELEMENT);
46694667
if(error_code != 0) {
46704668
session->mountpoint = NULL;
4669+
janus_mutex_unlock(&session->mutex);
46714670
janus_mutex_unlock(&mp->mutex);
46724671
janus_refcount_decrease(&mp->ref);
4673-
janus_mutex_unlock(&session->mutex);
46744672
goto error;
46754673
}
46764674
/* In case this mountpoint is simulcasting, let's aim high by default */
@@ -4706,9 +4704,9 @@ static void *janus_streaming_handler(void *data) {
47064704
JANUS_STREAMING_ERROR_MISSING_ELEMENT, JANUS_STREAMING_ERROR_INVALID_ELEMENT);
47074705
if(error_code != 0) {
47084706
session->mountpoint = NULL;
4707+
janus_mutex_unlock(&session->mutex);
47094708
janus_mutex_unlock(&mp->mutex);
47104709
janus_refcount_decrease(&mp->ref);
4711-
janus_mutex_unlock(&session->mutex);
47124710
goto error;
47134711
}
47144712
/* In case this mountpoint is doing VP9-SVC, let's aim high by default */
@@ -4845,8 +4843,8 @@ static void *janus_streaming_handler(void *data) {
48454843
}
48464844
}
48474845
}
4848-
janus_mutex_unlock(&mp->mutex);
48494846
janus_mutex_unlock(&session->mutex);
4847+
janus_mutex_unlock(&mp->mutex);
48504848
} else if(!strcasecmp(request_text, "start")) {
48514849
if(session->mountpoint == NULL) {
48524850
JANUS_LOG(LOG_VERB, "Can't start: no mountpoint set\n");
@@ -5077,6 +5075,8 @@ static void *janus_streaming_handler(void *data) {
50775075
JANUS_LOG(LOG_VERB, "Request to switch to mountpoint/stream %s (old: %s)\n", mp->id_str, oldmp->id_str);
50785076
session->paused = TRUE;
50795077
/* Unsubscribe from the previous mountpoint and subscribe to the new one */
5078+
session->mountpoint = NULL;
5079+
janus_mutex_unlock(&session->mutex);
50805080
janus_mutex_lock(&oldmp->mutex);
50815081
oldmp->viewers = g_list_remove_all(oldmp->viewers, session);
50825082
/* Remove the viewer from the helper threads too, if any */
@@ -5100,6 +5100,7 @@ static void *janus_streaming_handler(void *data) {
51005100
janus_mutex_unlock(&oldmp->mutex);
51015101
/* Subscribe to the new one */
51025102
janus_mutex_lock(&mp->mutex);
5103+
janus_mutex_lock(&session->mutex);
51035104
mp->viewers = g_list_append(mp->viewers, session);
51045105
/* If we're using helper threads, add the viewer to one of those */
51055106
if(mp->helper_threads > 0) {
@@ -5120,10 +5121,10 @@ static void *janus_streaming_handler(void *data) {
51205121
helper->num_viewers++;
51215122
janus_mutex_unlock(&helper->mutex);
51225123
}
5123-
janus_mutex_unlock(&mp->mutex);
51245124
session->mountpoint = mp;
51255125
session->paused = FALSE;
51265126
janus_mutex_unlock(&session->mutex);
5127+
janus_mutex_unlock(&mp->mutex);
51275128
/* Done */
51285129
janus_refcount_decrease(&oldmp->ref); /* This is for the request being done with it */
51295130
result = json_object();

0 commit comments

Comments
 (0)