app_confbridge: Race between removing and playing name recording while leaving

When user leaves a conference, its channel calls async_play_sound_file()
in order to play the name announcement and then unlinks the sound file.
The async_play_sound_file() function adds a task to conference playback queue,
which then runs playback_common() function in a different thread.

It leads to a race condition when, in some cases, channel thread may unlink
the sound file before playback_common() had a chance to open it.

This patch creates a file deletion task, that is queued after playback.

ASTERISK-27012 #close

Change-Id: I412f7922d412004b80917d4e892546c15bd70dd3

diff --git a/apps/app_confbridge.c b/apps/app_confbridge.c
index 6986fbe67e6f644ea2ee555cd62447bebc9260d9..e2a35ff3138bf5e033a5c11d4cb561edaa06a6f9 100644 (file)
--- a/apps/app_confbridge.c
+++ b/apps/app_confbridge.c
@@ -2146,6 +2146,80 @@ static int conf_rec_name(struct confbridge_user *user, const char *conf_name)
        return 0;
 }
 
+struct async_delete_name_rec_task_data {
+       struct confbridge_conference *conference;
+       char filename[0];
+};
+
+static struct async_delete_name_rec_task_data *async_delete_name_rec_task_data_alloc(
+       struct confbridge_conference *conference, const char *filename)
+{
+       struct async_delete_name_rec_task_data *atd;
+
+       atd = ast_malloc(sizeof(*atd) + strlen(filename) + 1);
+       if (!atd) {
+               return NULL;
+       }
+
+       /* Safe */
+       strcpy(atd->filename, filename);
+       atd->conference = conference;
+
+       return atd;
+}
+
+static void async_delete_name_rec_task_data_destroy(struct async_delete_name_rec_task_data *atd)
+{
+       ast_free(atd);
+}
+
+/*!
+ * \brief Delete user's name file asynchronously
+ *
+ * This runs in the playback queue taskprocessor. This ensures that
+ * sound file is removed after playback is finished and not before.
+ *
+ * \param data An async_delete_name_rec_task_data
+ * \return 0
+ */
+static int async_delete_name_rec_task(void *data)
+{
+       struct async_delete_name_rec_task_data *atd = data;
+
+       ast_filedelete(atd->filename, NULL);
+       ast_log(LOG_DEBUG, "Conference '%s' removed user name file '%s'\n",
+               atd->conference->name, atd->filename);
+
+       async_delete_name_rec_task_data_destroy(atd);
+       return 0;
+}
+
+static int async_delete_name_rec(struct confbridge_conference *conference,
+       const char *filename)
+{
+       struct async_delete_name_rec_task_data *atd;
+
+       if (ast_strlen_zero(filename)) {
+               return 0;
+       } else if (!sound_file_exists(filename)) {
+               return 0;
+       }
+
+       atd = async_delete_name_rec_task_data_alloc(conference, filename);
+       if (!atd) {
+               return -1;
+       }
+
+       if (ast_taskprocessor_push(conference->playback_queue, async_delete_name_rec_task, atd)) {
+               ast_log(LOG_WARNING, "Conference '%s' was unable to remove user name file '%s'\n",
+                       conference->name, filename);
+               async_delete_name_rec_task_data_destroy(atd);
+               return -1;
+       }
+
+       return 0;
+}
+
 static int join_callback(struct ast_bridge_channel *bridge_channel, void *ignore)
 {
        async_play_sound_ready(bridge_channel->chan);
@@ -2401,6 +2475,7 @@ static int confbridge_exec(struct ast_channel *chan, const char *data)
                async_play_sound_file(conference, user.name_rec_location, NULL);
                async_play_sound_file(conference,
                        conf_get_sound(CONF_SOUND_HAS_LEFT, conference->b_profile.sounds), NULL);
+               async_delete_name_rec(conference, user.name_rec_location);
        }
 
        /* play the leave sound */
@@ -2428,10 +2503,6 @@ static int confbridge_exec(struct ast_channel *chan, const char *data)
                ast_audiohook_volume_set(chan, AST_AUDIOHOOK_DIRECTION_WRITE, volume_adjustments[1]);
        }
 
-       if (!ast_strlen_zero(user.name_rec_location)) {
-               ast_filedelete(user.name_rec_location, NULL);
-       }
-
 confbridge_cleanup:
        ast_bridge_features_cleanup(&user.features);
        conf_bridge_profile_destroy(&user.b_profile);