From b214f2076889a95bbf7a70539f563b447047ff88 Mon Sep 17 00:00:00 2001 From: William King Date: Mon, 13 May 2013 15:10:03 -0700 Subject: [PATCH] Since there isn't neutral ground, on the rtmp state callback we actually have to force the write lock of the session, but we only need a try lock on removing the session from the rsession->session_hash. This removes a deadlock caused when we had to enforce with locks that the rsession couldn't be destroyed if the FS session was in the middle of a hangup. --- src/mod/endpoints/mod_rtmp/mod_rtmp.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/mod/endpoints/mod_rtmp/mod_rtmp.c b/src/mod/endpoints/mod_rtmp/mod_rtmp.c index 6e1c335d43..384cd05ccf 100644 --- a/src/mod/endpoints/mod_rtmp/mod_rtmp.c +++ b/src/mod/endpoints/mod_rtmp/mod_rtmp.c @@ -284,12 +284,17 @@ switch_status_t rtmp_on_hangup(switch_core_session_t *session) rtmp_notify_call_state(session); rtmp_send_onhangup(session); - switch_core_hash_delete_wrlock(rsession->session_hash, switch_core_session_get_uuid(session), rsession->session_rwlock); + /* + * If the session_rwlock is already locked, then there is a larger possibility that the rsession + * is looping through because the rsession is trying to hang them up. If that is the case, then there + * is really no reason to foce this hash_delete. Just timeout, and let the rsession handle the final cleanup + * since it now checks for the existance of the FS session safely. + */ + if ( switch_thread_rwlock_trywrlock_timeout(rsession->session_rwlock, 10) == SWITCH_STATUS_SUCCESS) { + switch_core_hash_delete(rsession->session_hash, switch_core_session_get_uuid(session)); + switch_thread_rwlock_unlock(rsession->session_rwlock); + } - switch_mutex_lock(rsession->count_mutex); - rsession->active_sessions--; - switch_mutex_unlock(rsession->count_mutex); - #ifndef RTMP_DONT_HOLD if (switch_channel_test_flag(channel, CF_HOLD)) { switch_channel_mark_hold(channel, SWITCH_FALSE); @@ -841,6 +846,13 @@ switch_status_t rtmp_session_destroy(rtmp_session_t **rsession) /* At this point we don't know if the session still exists, so request a fresh pointer to it from the core. */ if ( (session = switch_core_session_locate((char *)key)) != NULL ) { switch_core_session_rwunlock(session); + + /* + * This is here so that if the FS session still exists and has the FS session write(or read) lock, then we won't destroy the rsession + * until the FS session is finished with it. But if the rsession is able to get the FS session + * write lock, before the FS session is hungup, then once the FS session does get the write lock + * the rsession pointer will be null, and the FS session will never try and touch the already destroyed rsession. + */ switch_core_session_write_lock(session); channel = switch_core_session_get_channel(session); tech_pvt = switch_core_session_get_private(session); @@ -853,11 +865,6 @@ switch_status_t rtmp_session_destroy(rtmp_session_t **rsession) } switch_thread_rwlock_unlock((*rsession)->session_rwlock); - /* while ((*rsession)->active_sessions > 0) { - switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "Still have %d sessions, waiting\n", (*rsession)->active_sessions); - switch_yield(500000); - }*/ - switch_mutex_lock((*rsession)->profile->mutex); if ( (*rsession)->profile->calls < 1 ) { (*rsession)->profile->calls = 0;