Skip to content

Commit

Permalink
Fixes to leaks and race conditions in VoiceMail plugin (#1993)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lminiero
lminiero committed Mar 12, 2020
1 parent e45c3fe commit c78edb9
Showing 1 changed file with 24 additions and 19 deletions.
43 changes: 24 additions & 19 deletions plugins/janus_voicemail.c
View file
Expand Up @@ -205,8 +205,8 @@ typedef struct janus_voicemail_session {
FILE *file;
ogg_stream_state *stream;
int seq;
gboolean started;
gboolean stopping;
volatile gint started;
volatile gint stopping;
volatile gint hangingup;
volatile gint destroyed;
janus_refcount ref;
Expand All @@ -224,6 +224,9 @@ static void janus_voicemail_session_free(const janus_refcount *session_ref) {
/* Remove the reference to the core plugin session */
janus_refcount_decrease(&session->handle->ref);
/* This session can be destroyed, free all the resources */
g_free(session->filename);
if(session->file)
fclose(session->file);
g_free(session);
}
static void janus_voicemail_message_free(janus_voicemail_message *msg) {
Expand Down Expand Up @@ -442,8 +445,8 @@ void janus_voicemail_create_session(janus_plugin_session *handle, int *error) {
session->filename = g_strdup(f);
session->file = NULL;
session->seq = 0;
session->started = FALSE;
session->stopping = FALSE;
g_atomic_int_set(&session->started, 0);
g_atomic_int_set(&session->stopping, 0);
g_atomic_int_set(&session->hangingup, 0);
g_atomic_int_set(&session->destroyed, 0);
janus_refcount_init(&session->ref, janus_voicemail_session_free);
Expand Down Expand Up @@ -471,7 +474,6 @@ void janus_voicemail_destroy_session(janus_plugin_session *handle, int *error) {
}
JANUS_LOG(LOG_VERB, "Removing VoiceMail session...\n");
janus_voicemail_hangup_media_internal(handle);
handle->plugin_handle = NULL;
g_hash_table_remove(sessions, handle);
janus_mutex_unlock(&sessions_mutex);

Expand Down Expand Up @@ -550,7 +552,7 @@ void janus_voicemail_setup_media(janus_plugin_session *handle) {
g_atomic_int_set(&session->hangingup, 0);
/* Only start recording this peer when we get this event */
session->start_time = janus_get_monotonic_time();
session->started = TRUE;
g_atomic_int_set(&session->started, 1);
/* Prepare JSON event */
json_t *event = json_object();
json_object_set_new(event, "voicemail", json_string("event"));
Expand All @@ -565,13 +567,14 @@ void janus_voicemail_incoming_rtp(janus_plugin_session *handle, janus_plugin_rtp
if(handle == NULL || g_atomic_int_get(&handle->stopped) || g_atomic_int_get(&stopping) || !g_atomic_int_get(&initialized))
return;
janus_voicemail_session *session = (janus_voicemail_session *)handle->plugin_handle;
if(!session || g_atomic_int_get(&session->destroyed) || session->stopping || !session->started || session->start_time == 0)
if(!session || g_atomic_int_get(&session->destroyed) || g_atomic_int_get(&session->stopping) ||
!g_atomic_int_get(&session->started) || session->start_time == 0)
return;
gint64 now = janus_get_monotonic_time();
/* Have 10 seconds passed? */
if((now-session->start_time) >= 10*G_USEC_PER_SEC) {
/* FIXME Simulate a "stop" coming from the browser */
session->started = FALSE;
g_atomic_int_set(&session->started, 0);
janus_refcount_increase(&session->ref);
janus_voicemail_message *msg = g_malloc(sizeof(janus_voicemail_message));
msg->handle = handle;
Expand Down Expand Up @@ -623,7 +626,7 @@ static void janus_voicemail_hangup_media_internal(janus_plugin_session *handle)
JANUS_LOG(LOG_ERR, "No session associated with this handle...\n");
return;
}
session->started = FALSE;
g_atomic_int_set(&session->started, 0);
if(g_atomic_int_get(&session->destroyed))
return;
if(!g_atomic_int_compare_and_exchange(&session->hangingup, 0, 1))
Expand Down Expand Up @@ -728,7 +731,7 @@ static void *janus_voicemail_handler(void *data) {
/* Done: now wait for the setup_media callback to be called */
event = json_object();
json_object_set_new(event, "voicemail", json_string("event"));
json_object_set_new(event, "status", json_string(session->started ? "started" : "starting"));
json_object_set_new(event, "status", json_string(g_atomic_int_get(&session->started) ? "started" : "starting"));
/* Also notify event handlers */
if(notify_events && gateway->events_is_enabled()) {
json_t *info = json_object();
Expand All @@ -738,7 +741,7 @@ static void *janus_voicemail_handler(void *data) {
} else if(!strcasecmp(request_text, "update")) {
/* Only needed in case of renegotiations and ICE restarts (but with 10s messages is this worth it?) */
JANUS_LOG(LOG_VERB, "Updating existing recording\n");
if(session->stream == NULL || !session->started) {
if(session->stream == NULL || !g_atomic_int_get(&session->started)) {
JANUS_LOG(LOG_ERR, "Invalid state (not recording)\n");
error_code = JANUS_VOICEMAIL_ERROR_INVALID_STATE;
g_snprintf(error_cause, 512, "Invalid state (not recording)");
Expand All @@ -750,8 +753,8 @@ static void *janus_voicemail_handler(void *data) {
json_object_set_new(event, "status", json_string("updating"));
} else if(!strcasecmp(request_text, "stop")) {
/* Stop the recording */
session->started = FALSE;
session->stopping = TRUE;
g_atomic_int_set(&session->started, 0);
g_atomic_int_set(&session->stopping, 1);
if(session->file)
fclose(session->file);
session->file = NULL;
Expand All @@ -762,9 +765,11 @@ static void *janus_voicemail_handler(void *data) {
event = json_object();
json_object_set_new(event, "voicemail", json_string("event"));
json_object_set_new(event, "status", json_string("done"));
char url[1024];
g_snprintf(url, 1024, "%s/janus-voicemail-%"SCNu64".opus", recordings_base, session->recording_id);
json_object_set_new(event, "recording", json_string(url));
if(session->recording_id > 0) {
char url[1024];
g_snprintf(url, 1024, "%s/janus-voicemail-%"SCNu64".opus", recordings_base, session->recording_id);
json_object_set_new(event, "recording", json_string(url));
}
/* Also notify event handlers */
if(notify_events && gateway->events_is_enabled()) {
json_t *info = json_object();
Expand Down Expand Up @@ -831,11 +836,11 @@ static void *janus_voicemail_handler(void *data) {
/* TODO Failed to negotiate? We should remove this participant */
}
}
janus_voicemail_message_free(msg);

if(session->stopping) {
/* Tear down the session if we're done */
if(g_atomic_int_get(&session->stopping))
gateway->end_session(session->handle);
}
janus_voicemail_message_free(msg);

continue;

Expand Down

0 comments on commit c78edb9

Please sign in to comment.