]> git.ipfire.org Git - thirdparty/freeswitch.git/commitdiff
[Core] Fix hangup race in recording_follow_transfer. 802/head
authorAndrey Volk <andywolk@gmail.com>
Wed, 1 Jul 2020 00:19:25 +0000 (04:19 +0400)
committerAndrey Volk <andywolk@gmail.com>
Mon, 17 Aug 2020 17:07:11 +0000 (21:07 +0400)
src/include/switch_core.h
src/switch_core_file.c
src/switch_core_media_bug.c
src/switch_ivr_async.c

index cad1e0cb17f315f5cfaa0b399a91b4427f25ea38..fba42b768e141b80460dba3ca1fcf7d06351b44c 100644 (file)
@@ -1973,6 +1973,16 @@ SWITCH_DECLARE(switch_status_t) switch_core_file_get_string(_In_ switch_file_han
 */
 SWITCH_DECLARE(switch_status_t) switch_core_file_pre_close(_In_ switch_file_handle_t *fh);
 
+/*!
+  \brief Duplicates a file handle using another pool
+  \param oldfh the file handle to duplicate
+  \param newfh pointer to assign new file handle to
+  \param pool the pool to use (NULL for new pool)
+  \return SWITCH_STATUS_SUCCESS if the file handle was duplicated
+*/
+
+SWITCH_DECLARE(switch_status_t) switch_core_file_handle_dup(switch_file_handle_t *oldfh, switch_file_handle_t **newfh, switch_memory_pool_t *pool);
+
 /*!
   \brief Close an open file handle
   \param fh the file handle to close
index abeda29430d1174b1fc8b5252728c0dc4b0851cb..5398372a213dc6bb52f17707476346422aecc678 100644 (file)
@@ -940,6 +940,75 @@ SWITCH_DECLARE(switch_status_t) switch_core_file_pre_close(switch_file_handle_t
        return status;
 }
 
+SWITCH_DECLARE(switch_status_t) switch_core_file_handle_dup(switch_file_handle_t *oldfh, switch_file_handle_t **newfh, switch_memory_pool_t *pool)
+{
+       switch_status_t status;
+       switch_file_handle_t *fh;
+       uint8_t destroy_pool = 0;
+
+       switch_assert(oldfh != NULL);
+       switch_assert(newfh != NULL);
+
+       if (!pool) {
+               if ((status = switch_core_new_memory_pool(&pool)) != SWITCH_STATUS_SUCCESS) {
+                       return status;
+               }
+
+               destroy_pool = 1;
+       }
+
+       if (!(fh = switch_core_alloc(pool, sizeof(switch_file_handle_t)))) {
+               switch_goto_status(SWITCH_STATUS_MEMERR, err);
+       }
+
+       memcpy(fh, oldfh, sizeof(switch_file_handle_t));
+
+       if (!destroy_pool) {
+               switch_clear_flag(fh, SWITCH_FILE_FLAG_FREE_POOL);
+       } else {
+               fh->memory_pool = pool;
+               switch_set_flag(fh, SWITCH_FILE_FLAG_FREE_POOL);
+       }
+
+       if ((status = switch_mutex_init(&fh->flag_mutex, SWITCH_MUTEX_NESTED, pool)) != SWITCH_STATUS_SUCCESS) {
+               switch_goto_status(status, err);
+       }
+
+#define DUP_CHECK(dup) if (oldfh->dup && !(fh->dup = switch_core_strdup(pool, oldfh->dup))) {switch_goto_status(SWITCH_STATUS_MEMERR, err);}
+
+       DUP_CHECK(prefix);
+       DUP_CHECK(modname);
+       DUP_CHECK(mm.auth_username);
+       DUP_CHECK(mm.auth_password);
+       DUP_CHECK(stream_name);
+       DUP_CHECK(file_path);
+       DUP_CHECK(handler);
+       DUP_CHECK(spool_path);
+       
+       fh->pre_buffer_data = NULL;
+       if (oldfh->pre_buffer_data) {
+               switch_size_t pre_buffer_data_size = oldfh->pre_buffer_datalen * oldfh->channels;
+               if (pre_buffer_data_size) {
+                       if (!(fh->pre_buffer_data = switch_core_alloc(pool, pre_buffer_data_size))) {
+                               switch_goto_status(SWITCH_STATUS_MEMERR, err);
+                       }
+
+                       memcpy(fh->pre_buffer_data, oldfh->pre_buffer_data, pre_buffer_data_size);
+               }
+       }
+
+       *newfh = fh;
+
+       return SWITCH_STATUS_SUCCESS;
+
+err:
+       if (destroy_pool) {
+               switch_core_destroy_memory_pool(&pool);
+       }
+
+       return status;
+}
+
 SWITCH_DECLARE(switch_status_t) switch_core_file_close(switch_file_handle_t *fh)
 {
        switch_status_t status = SWITCH_STATUS_SUCCESS;
index a229f611e9d7c85dbfd5c592fad18929fc88f7a9..4e92c2d9e66819fb84540de9d6568c9adf1d8f43 100644 (file)
@@ -1038,7 +1038,7 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_bug_flush_all(switch_core_sess
 SWITCH_DECLARE(switch_status_t) switch_core_media_bug_transfer_callback(switch_core_session_t *orig_session, switch_core_session_t *new_session,
                                                                                                                                                switch_media_bug_callback_t callback, void * (*user_data_dup_func) (switch_core_session_t *, void *))
 {
-       switch_media_bug_t *new_bug = NULL, *cur = NULL, *bp = NULL, *last = NULL;
+       switch_media_bug_t *new_bug = NULL, *cur = NULL, *bp = NULL, *last = NULL, *old_last_next = NULL, *old_bugs = NULL;
        int total = 0;
 
        if (!switch_channel_media_ready(new_session->channel)) {
@@ -1054,19 +1054,36 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_bug_transfer_callback(switch_c
 
                if (cur->callback == callback) {
                        if (last) {
+                               old_last_next = last->next;
                                last->next = cur->next;
                        } else {
+                               old_bugs = orig_session->bugs;
                                orig_session->bugs = cur->next;
                        }
 
                        switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(orig_session), SWITCH_LOG_DEBUG, "Transfering %s from %s to %s\n", cur->target,
                                                          switch_core_session_get_name(orig_session), switch_core_session_get_name(new_session));
 
-                       switch_core_media_bug_add(new_session, cur->function, cur->target, cur->callback,
-                                                                         user_data_dup_func(new_session, cur->user_data),
-                                                                         cur->stop_time, cur->flags, &new_bug);
-                       switch_core_media_bug_destroy(&cur);
-                       total++;
+                       if ((switch_core_media_bug_add(new_session, cur->function, cur->target, cur->callback,
+                                                                                  user_data_dup_func(new_session, cur->user_data),
+                                                                                  cur->stop_time, cur->flags, &new_bug) == SWITCH_STATUS_SUCCESS)) {
+                               switch_core_media_bug_destroy(&cur);
+                               total++;
+                       } else {
+                               /* Call the dup function again to revert to original session */
+                               user_data_dup_func(orig_session, cur->user_data);
+                               switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(orig_session), SWITCH_LOG_DEBUG, "Adding a bug failed: abort transfering %s from %s to %s\n", cur->target,
+                                       switch_core_session_get_name(orig_session), switch_core_session_get_name(new_session));
+
+                               /* Put the old bug back to the original session's list of bugs */
+                               if (last) {
+                                       last->next = old_last_next;
+                               } else {
+                                       orig_session->bugs = old_bugs;
+                               }
+
+                               last = cur;
+                       }
                } else {
                        last = cur;
                }
index 14d6a21c154f29e7acb7f628d1128252ab60ed78..705142228cc5d3b580320646a137a5aadb4e6f1c 100644 (file)
@@ -1123,6 +1123,11 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_displace_session(switch_core_session_
 
 
 struct record_helper {
+       switch_media_bug_t *bug;
+       switch_memory_pool_t *helper_pool;
+       switch_core_session_t *recording_session;
+       switch_core_session_t *transfer_from_session;
+       uint8_t transfer_complete;
        char *file;
        switch_file_handle_t *fh;
        switch_file_handle_t in_fh;
@@ -1146,6 +1151,7 @@ struct record_helper {
        switch_thread_t *thread;
        switch_mutex_t *buffer_mutex;
        int thread_ready;
+       uint8_t thread_needs_transfer;
        uint32_t writes;
        uint32_t vwrites;
        const char *completion_cause;
@@ -1153,6 +1159,8 @@ struct record_helper {
        switch_event_t *variables;
 };
 
+static switch_status_t record_helper_destroy(struct record_helper **rh, switch_core_session_t *session);
+
 /**
  * Set the recording completion cause. The cause can only be set once, to minimize the logic in the record_callback.
  * [The completion_cause strings are essentially those of an MRCP Recorder resource.]
@@ -1257,9 +1265,29 @@ static void *SWITCH_THREAD_FUNC recording_thread(switch_thread_t *thread, void *
        rh->thread_ready = 1;
 
        channels = switch_core_media_bug_test_flag(bug, SMBF_STEREO) ? 2 : rh->read_impl.number_of_channels;
-       data = switch_core_session_alloc(session, SWITCH_RECOMMENDED_BUFFER_SIZE);
+       data = switch_core_alloc(rh->helper_pool, SWITCH_RECOMMENDED_BUFFER_SIZE);
 
        while(switch_test_flag(rh->fh, SWITCH_FILE_OPEN)) {
+               if (rh->thread_needs_transfer) {
+                       assert(session != rh->recording_session);
+
+                       if (switch_core_session_read_lock(rh->recording_session) != SWITCH_STATUS_SUCCESS) {
+                               continue;
+                       }
+
+                       switch_core_session_rwunlock(session);
+                       session = rh->recording_session;
+                       channel = switch_core_session_get_channel(session);
+                       bug = rh->bug;
+                       channels = switch_core_media_bug_test_flag(bug, SMBF_STEREO) ? 2 : rh->read_impl.number_of_channels;
+
+                       /* Tell record_callback that we transferred */
+                       rh->thread_needs_transfer = 0;
+
+                       /* Erasing the obj variable for safety because we transferred (we are under another bug) */
+                       obj = NULL;
+               }
+
                if (switch_core_file_has_video(rh->fh, SWITCH_TRUE)) {
                        switch_core_session_get_read_impl(session, &read_impl);
                        if (read_impl.decoded_bytes_per_packet > 0 && read_impl.decoded_bytes_per_packet <= SWITCH_RECOMMENDED_BUFFER_SIZE) {
@@ -1309,22 +1337,71 @@ static switch_bool_t record_callback(switch_media_bug_t *bug, void *user_data, s
        int mask = switch_core_media_bug_test_flag(bug, SMBF_MASK);
        unsigned char null_data[SWITCH_RECOMMENDED_BUFFER_SIZE] = {0};
 
+       /* Check if the recording was transferred (see recording_follow_transfer) */
+       if (rh->recording_session != session) {
+               return SWITCH_FALSE;
+       }
+
        switch (type) {
        case SWITCH_ABC_TYPE_INIT:
                {
                        const char *var = switch_channel_get_variable(channel, "RECORD_USE_THREAD");
 
+                       switch_core_session_get_read_impl(session, &rh->read_impl);
+
+                       /* Check if recording is transferred from another session */
+                       if (rh->transfer_from_session && rh->transfer_from_session != rh->recording_session) {
+
+                               /* If there is a thread, we need to re-initiate it */
+                               if (rh->thread_ready) {
+                                       switch_media_bug_t *oldbug = rh->bug;
+                                       int sanity = 200;
+
+                                       switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_DEBUG, "Re-initiating recording thread for file %s\n", rh->file);
+                                       
+                                       rh->bug = bug;
+                                       rh->thread_needs_transfer = 1;
+
+                                       while (--sanity > 0 && rh->thread_needs_transfer) {
+                                               switch_yield(10000);
+                                       }
+
+                                       /* Check if the thread reacted on the transfer */
+                                       if (rh->thread_needs_transfer) {
+                                               /* Thread did not react, assuming it is cond_wait'ing */
+                                               rh->bug = oldbug;
+                                               switch_core_session_get_read_impl(rh->transfer_from_session, &rh->read_impl);
+                                               rh->thread_needs_transfer = 0;
+
+                                               return SWITCH_FALSE;
+                                       }
+                               }
+
+                               if (rh->fh) {
+                                       switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_DEBUG, "Record session sample rate: %d -> %d\n", rh->fh->native_rate, rh->fh->samplerate);
+                                       rh->fh->native_rate = rh->read_impl.actual_samples_per_second;
+                                       if (switch_core_file_has_video(rh->fh, SWITCH_TRUE)) {
+                                               switch_core_media_bug_set_media_params(bug, &rh->fh->mm);
+                                       }
+                               }
+
+                               rh->transfer_from_session = NULL;
+                               rh->transfer_complete = 1;
+
+                               break;
+                       }
+
+                       /* Required for potential record_transfer */
+                       rh->bug = bug;
+                       
                        if (!rh->native && rh->fh && (zstr(var) || switch_true(var))) {
                                switch_threadattr_t *thd_attr = NULL;
-                               switch_memory_pool_t *pool = switch_core_session_get_pool(session);
                                int sanity = 200;
 
-
-                               switch_core_session_get_read_impl(session, &rh->read_impl);
-                               switch_mutex_init(&rh->buffer_mutex, SWITCH_MUTEX_NESTED, pool);
-                               switch_threadattr_create(&thd_attr, pool);
+                               switch_mutex_init(&rh->buffer_mutex, SWITCH_MUTEX_NESTED, rh->helper_pool);
+                               switch_threadattr_create(&thd_attr, rh->helper_pool);
                                switch_threadattr_stacksize_set(thd_attr, SWITCH_THREAD_STACKSIZE);
-                               switch_thread_create(&rh->thread, thd_attr, recording_thread, bug, pool);
+                               switch_thread_create(&rh->thread, thd_attr, recording_thread, bug, rh->helper_pool);
 
                                while(--sanity > 0 && !rh->thread_ready) {
                                        switch_yield(10000);
@@ -1346,7 +1423,6 @@ static switch_bool_t record_callback(switch_media_bug_t *bug, void *user_data, s
                        rh->speech_detected = SWITCH_FALSE;
                        rh->completion_cause = NULL;
 
-                       switch_core_session_get_read_impl(session, &rh->read_impl);
                        if (rh->fh) {
                                switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_DEBUG, "Record session sample rate: %d -> %d\n", rh->fh->native_rate, rh->fh->samplerate);
                                rh->fh->native_rate = rh->read_impl.actual_samples_per_second;
@@ -1354,7 +1430,6 @@ static switch_bool_t record_callback(switch_media_bug_t *bug, void *user_data, s
                                        switch_core_media_bug_set_media_params(bug, &rh->fh->mm);
                                }
                        }
-
                }
                break;
        case SWITCH_ABC_TYPE_TAP_NATIVE_READ:
@@ -1483,6 +1558,8 @@ static switch_bool_t record_callback(switch_media_bug_t *bug, void *user_data, s
                                                        switch_core_session_reset(session, SWITCH_TRUE, SWITCH_TRUE);
                                                }
                                                send_record_stop_event(channel, &read_impl, rh);
+                                               record_helper_destroy(&rh, session);
+
                                                return SWITCH_FALSE;
                                        }
                                }
@@ -1558,7 +1635,7 @@ static switch_bool_t record_callback(switch_media_bug_t *bug, void *user_data, s
 
                        }
 
-
+                       record_helper_destroy(&rh, session);
                }
 
                break;
@@ -1779,20 +1856,19 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_stop_record_session(switch_core_sessi
 
 static void* switch_ivr_record_user_data_dup(switch_core_session_t *session, void *user_data)
 {
-       struct record_helper *rh = (struct record_helper *) user_data, *dup = NULL;
-
-       dup = switch_core_session_alloc(session, sizeof(*dup));
-       memcpy(dup, rh, sizeof(*rh));
-       dup->file = switch_core_session_strdup(session, rh->file);
-       dup->fh = switch_core_session_alloc(session, sizeof(switch_file_handle_t));
-       memcpy(dup->fh, rh->fh, sizeof(switch_file_handle_t));
-       dup->variables = NULL;
-       if (rh->variables) {
-               switch_event_dup(&dup->variables, rh->variables);
-               switch_event_safe_destroy(rh->variables);
+       struct record_helper *rh = (struct record_helper *) user_data;
+
+       if (!rh->transfer_complete && session == rh->transfer_from_session) {
+               /* Transfer failed and now we are called to put the original session back */
+               rh->recording_session = rh->transfer_from_session;
+               rh->transfer_from_session = NULL;
+       } else {
+               rh->transfer_from_session = rh->recording_session;
+               rh->recording_session = session;
+               rh->transfer_complete = 0;
        }
 
-       return dup;
+       return rh;
 }
 
 SWITCH_DECLARE(switch_status_t) switch_ivr_transfer_recordings(switch_core_session_t *orig_session, switch_core_session_t *new_session)
@@ -2613,6 +2689,58 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_eavesdrop_session(switch_core_session
        return status;
 }
 
+static switch_status_t record_helper_create(struct record_helper **rh, switch_core_session_t *session)
+{
+       switch_status_t status;
+       switch_memory_pool_t *pool;
+       struct record_helper *newrh;
+
+       assert(rh);
+       assert(session);
+
+       if ((status = switch_core_new_memory_pool(&pool)) != SWITCH_STATUS_SUCCESS) {
+               return status;
+       }
+
+       if (!(newrh = switch_core_alloc(pool, sizeof(*newrh)))) {
+               switch_core_destroy_memory_pool(&pool);
+               return SWITCH_STATUS_MEMERR;
+       }
+
+       newrh->helper_pool = pool;
+       newrh->recording_session = session;
+
+       *rh = newrh;
+
+       return SWITCH_STATUS_SUCCESS;
+}
+
+static switch_status_t record_helper_destroy(struct record_helper **rh, switch_core_session_t *session)
+{
+       switch_memory_pool_t *pool;
+
+       assert(rh);
+       assert(*rh);
+       assert(session);
+
+       if ((*rh)->recording_session != session) {
+               switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_ERROR, "Destroying a record helper of another session!\n");
+       }
+
+       if ((*rh)->native) {
+               switch_core_file_close(&(*rh)->in_fh);
+               switch_core_file_close(&(*rh)->out_fh);
+       } else if((*rh)->fh) {
+               switch_core_file_close((*rh)->fh);
+       }
+
+       pool = (*rh)->helper_pool;
+       switch_core_destroy_memory_pool(&pool);
+       *rh = NULL;
+
+       return SWITCH_STATUS_SUCCESS;
+}
+
 SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_session_t *session, const char *file, uint32_t limit, switch_file_handle_t *fh, switch_event_t *vars)
 {
        switch_channel_t *channel = switch_core_session_get_channel(session);
@@ -2648,7 +2776,7 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
        channels = read_impl.number_of_channels;
 
        if ((bug = switch_channel_get_private(channel, file))) {
-               if (switch_true(switch_channel_get_variable(channel, "RECORD_TOGGLE_ON_REPEAT"))) {
+               if (switch_channel_var_true(channel, "RECORD_TOGGLE_ON_REPEAT")) {
                        return switch_ivr_stop_record_session(session, file);
                }
 
@@ -2657,7 +2785,7 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
        }
 
 
-       if ((p = switch_channel_get_variable(channel, "RECORD_CHECK_BRIDGE")) && switch_true(p)) {
+       if (switch_channel_var_true(channel, "RECORD_CHECK_BRIDGE")) {
                switch_core_session_t *other_session;
                int exist = 0;
                switch_status_t rstatus = SWITCH_STATUS_SUCCESS;
@@ -2680,45 +2808,57 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
                }
        }
 
-       if (!fh) {
-               if (!(fh = switch_core_session_alloc(session, sizeof(*fh)))) {
-                       return SWITCH_STATUS_MEMERR;
+       if ((status = record_helper_create(&rh, session)) != SWITCH_STATUS_SUCCESS) {
+               return status;
+       }
+
+       if (fh) {
+               switch_file_handle_t *newfh;
+
+               if ((status = switch_core_file_handle_dup(fh, &newfh, rh->helper_pool)) != SWITCH_STATUS_SUCCESS) {
+                       switch_goto_status(status, err);
+               }
+
+               fh = newfh;
+       } else {
+               if (!(fh = switch_core_alloc(rh->helper_pool, sizeof(*fh)))) {
+                       switch_goto_status(SWITCH_STATUS_MEMERR, err);
                }
        }
 
-       if ((p = switch_channel_get_variable(channel, "RECORD_WRITE_ONLY")) && switch_true(p)) {
+       if (switch_channel_var_true(channel, "RECORD_WRITE_ONLY")) {
                flags &= ~SMBF_READ_STREAM;
                flags |= SMBF_WRITE_STREAM;
        }
 
-       if ((p = switch_channel_get_variable(channel, "RECORD_READ_ONLY")) && switch_true(p)) {
+       if (switch_channel_var_true(channel, "RECORD_READ_ONLY")) {
                flags &= ~SMBF_WRITE_STREAM;
                flags |= SMBF_READ_STREAM;
        }
 
        if (channels == 1) { /* if leg is already stereo this feature is not available */
-               if ((p = switch_channel_get_variable(channel, "RECORD_STEREO")) && switch_true(p)) {
+               if (switch_channel_var_true(channel, "RECORD_STEREO")) {
                        flags |= SMBF_STEREO;
                        flags &= ~SMBF_STEREO_SWAP;
                        channels = 2;
                }
 
-               if ((p = switch_channel_get_variable(channel, "RECORD_STEREO_SWAP")) && switch_true(p)) {
+               if (switch_channel_var_true(channel, "RECORD_STEREO_SWAP")) {
                        flags |= SMBF_STEREO;
                        flags |= SMBF_STEREO_SWAP;
                        channels = 2;
                }
        }
 
-       if ((p = switch_channel_get_variable(channel, "RECORD_ANSWER_REQ")) && switch_true(p)) {
+       if (switch_channel_var_true(channel, "RECORD_ANSWER_REQ")) {
                flags |= SMBF_ANSWER_REQ;
        }
 
-       if ((p = switch_channel_get_variable(channel, "RECORD_BRIDGE_REQ")) && switch_true(p)) {
+       if (switch_channel_var_true(channel, "RECORD_BRIDGE_REQ")) {
                flags |= SMBF_BRIDGE_REQ;
        }
 
-       if ((p = switch_channel_get_variable(channel, "RECORD_APPEND")) && switch_true(p)) {
+       if (switch_channel_var_true(channel, "RECORD_APPEND")) {
                file_flags |= SWITCH_FILE_WRITE_APPEND;
        }
 
@@ -2739,8 +2879,6 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
 
        fh->channels = channels;
 
-
-
        if ((vval = switch_channel_get_variable(channel, "enable_file_write_buffering"))) {
                int tmp = atoi(vval);
 
@@ -2754,7 +2892,6 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
                fh->pre_buffer_datalen = SWITCH_DEFAULT_FILE_BUFFER_LEN;
        }
 
-
        if (!switch_is_file_path(file)) {
                char *tfile = NULL;
                char *e;
@@ -2767,7 +2904,7 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
                }
 
                if (*file == '[') {
-                       tfile = switch_core_session_strdup(session, file);
+                       tfile = switch_core_strdup(rh->helper_pool, file);
                        if ((e = switch_find_end_paren(tfile, '[', ']'))) {
                                *e = '\0';
                                file = e + 1;
@@ -2775,17 +2912,17 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
                                tfile = NULL;
                        }
                } else {
-                       file_path = switch_core_session_sprintf(session, "%s%s%s", prefix, SWITCH_PATH_SEPARATOR, file);
+                       file_path = switch_core_sprintf(rh->helper_pool, "%s%s%s", prefix, SWITCH_PATH_SEPARATOR, file);
                }
 
-               file = switch_core_session_sprintf(session, "%s%s%s%s%s", switch_str_nil(tfile), tfile ? "]" : "", prefix, SWITCH_PATH_SEPARATOR, file);
+               file = switch_core_sprintf(rh->helper_pool, "%s%s%s%s%s", switch_str_nil(tfile), tfile ? "]" : "", prefix, SWITCH_PATH_SEPARATOR, file);
        } else {
-               file_path = switch_core_session_strdup(session, file);
+               file_path = switch_core_strdup(rh->helper_pool, file);
        }
 
        if (file_path && !strstr(file_path, SWITCH_URL_SEPARATOR)) {
                char *p;
-               char *path = switch_core_session_strdup(session, file_path);
+               char *path = switch_core_strdup(rh->helper_pool, file_path);
 
                if ((p = strrchr(path, *SWITCH_PATH_SEPARATOR))) {
                        *p = '\0';
@@ -2796,7 +2933,7 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
 
                        if (switch_dir_make_recursive(path, SWITCH_DEFAULT_DIR_PERMS, switch_core_session_get_pool(session)) != SWITCH_STATUS_SUCCESS) {
                                switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_ERROR, "Error creating %s\n", path);
-                               return SWITCH_STATUS_GENERR;
+                               switch_goto_status(SWITCH_STATUS_GENERR, err);
                        }
 
                } else {
@@ -2805,8 +2942,6 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
                }
        }
 
-       rh = switch_core_session_alloc(session, sizeof(*rh));
-
        if ((ext = strrchr(file, '.'))) {
                ext++;
 
@@ -2820,7 +2955,7 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
                                switch_channel_hangup(channel, SWITCH_CAUSE_DESTINATION_OUT_OF_ORDER);
                                switch_core_session_reset(session, SWITCH_TRUE, SWITCH_TRUE);
                        }
-                       return SWITCH_STATUS_GENERR;
+                       switch_goto_status(SWITCH_STATUS_GENERR, err);
                }
 
                if (fh->params) {
@@ -2859,8 +2994,8 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
 
                ext = read_impl.iananame;
 
-               in_file = switch_core_session_sprintf(session, "%s-in.%s", file, ext);
-               out_file = switch_core_session_sprintf(session, "%s-out.%s", file, ext);
+               in_file = switch_core_sprintf(rh->helper_pool, "%s-in.%s", file, ext);
+               out_file = switch_core_sprintf(rh->helper_pool, "%s-out.%s", file, ext);
                rh->in_fh.pre_buffer_datalen = rh->out_fh.pre_buffer_datalen = fh->pre_buffer_datalen;
                channels = 1;
                switch_set_flag(&rh->in_fh, SWITCH_FILE_NATIVE);
@@ -2872,7 +3007,7 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
                                switch_channel_hangup(channel, SWITCH_CAUSE_DESTINATION_OUT_OF_ORDER);
                                switch_core_session_reset(session, SWITCH_TRUE, SWITCH_TRUE);
                        }
-                       return SWITCH_STATUS_GENERR;
+                       switch_goto_status(SWITCH_STATUS_GENERR, err);
                }
 
                if (switch_core_file_open(&rh->out_fh, out_file, channels, read_impl.actual_samples_per_second, file_flags, NULL) != SWITCH_STATUS_SUCCESS) {
@@ -2882,7 +3017,7 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
                                switch_channel_hangup(channel, SWITCH_CAUSE_DESTINATION_OUT_OF_ORDER);
                                switch_core_session_reset(session, SWITCH_TRUE, SWITCH_TRUE);
                        }
-                       return SWITCH_STATUS_GENERR;
+                       switch_goto_status(SWITCH_STATUS_GENERR, err);
                }
 
                rh->native = 1;
@@ -2899,40 +3034,38 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
                flags = tflags;
        }
 
-
-
        if ((p = switch_channel_get_variable(channel, "RECORD_TITLE"))) {
-               vval = (const char *) switch_core_session_strdup(session, p);
+               vval = (const char *) switch_core_strdup(rh->helper_pool, p);
                if (fh) switch_core_file_set_string(fh, SWITCH_AUDIO_COL_STR_TITLE, vval);
                switch_channel_set_variable(channel, "RECORD_TITLE", NULL);
        }
 
        if ((p = switch_channel_get_variable(channel, "RECORD_COPYRIGHT"))) {
-               vval = (const char *) switch_core_session_strdup(session, p);
+               vval = (const char *) switch_core_strdup(rh->helper_pool, p);
                if (fh) switch_core_file_set_string(fh, SWITCH_AUDIO_COL_STR_COPYRIGHT, vval);
                switch_channel_set_variable(channel, "RECORD_COPYRIGHT", NULL);
        }
 
        if ((p = switch_channel_get_variable(channel, "RECORD_SOFTWARE"))) {
-               vval = (const char *) switch_core_session_strdup(session, p);
+               vval = (const char *) switch_core_strdup(rh->helper_pool, p);
                if (fh) switch_core_file_set_string(fh, SWITCH_AUDIO_COL_STR_SOFTWARE, vval);
                switch_channel_set_variable(channel, "RECORD_SOFTWARE", NULL);
        }
 
        if ((p = switch_channel_get_variable(channel, "RECORD_ARTIST"))) {
-               vval = (const char *) switch_core_session_strdup(session, p);
+               vval = (const char *) switch_core_strdup(rh->helper_pool, p);
                if (fh) switch_core_file_set_string(fh, SWITCH_AUDIO_COL_STR_ARTIST, vval);
                switch_channel_set_variable(channel, "RECORD_ARTIST", NULL);
        }
 
        if ((p = switch_channel_get_variable(channel, "RECORD_COMMENT"))) {
-               vval = (const char *) switch_core_session_strdup(session, p);
+               vval = (const char *) switch_core_strdup(rh->helper_pool, p);
                if (fh) switch_core_file_set_string(fh, SWITCH_AUDIO_COL_STR_COMMENT, vval);
                switch_channel_set_variable(channel, "RECORD_COMMENT", NULL);
        }
 
        if ((p = switch_channel_get_variable(channel, "RECORD_DATE"))) {
-               vval = (const char *) switch_core_session_strdup(session, p);
+               vval = (const char *) switch_core_strdup(rh->helper_pool, p);
                if (fh) switch_core_file_set_string(fh, SWITCH_AUDIO_COL_STR_DATE, vval);
                switch_channel_set_variable(channel, "RECORD_DATE", NULL);
        }
@@ -2942,7 +3075,7 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
        }
 
        rh->fh = fh;
-       rh->file = switch_core_session_strdup(session, file);
+       rh->file = switch_core_strdup(rh->helper_pool, file);
        rh->packet_len = read_impl.decoded_bytes_per_packet;
 
        if (file_flags & SWITCH_FILE_WRITE_APPEND) {
@@ -2979,7 +3112,7 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
                }
        }
 
-       if(vars) {
+       if (vars) {
                switch_event_dup(&rh->variables, vars);
        }
 
@@ -2988,13 +3121,7 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
        if ((status = switch_core_media_bug_add(session, "session_record", file,
                                                                                        record_callback, rh, to, flags, &bug)) != SWITCH_STATUS_SUCCESS) {
                switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_ERROR, "Error adding media bug for file %s\n", file);
-               if (rh->native) {
-                       switch_core_file_close(&rh->in_fh);
-                       switch_core_file_close(&rh->out_fh);
-               } else {
-                       switch_core_file_close(fh);
-               }
-               return status;
+               switch_goto_status(status, err);
        }
 
        if ((p = switch_channel_get_variable(channel, "RECORD_PRE_BUFFER_FRAMES"))) {
@@ -3019,6 +3146,11 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_record_session_event(switch_core_sess
        }
 
        return SWITCH_STATUS_SUCCESS;
+
+err:
+       record_helper_destroy(&rh, session);
+
+       return status;
 }
 
 SWITCH_DECLARE(switch_status_t) switch_ivr_record_session(switch_core_session_t *session, const char *file, uint32_t limit, switch_file_handle_t *fh)