From c20bce98709ec837fb7dec27f7b55fbcae7305ab Mon Sep 17 00:00:00 2001 From: winlin Date: Fri, 5 Sep 2025 10:03:39 -0400 Subject: [PATCH] RTX: Fix race condition for timer. v6.0.176 (#4470) (#4474) --- trunk/doc/CHANGELOG.md | 1 + trunk/src/app/srs_app_rtc_conn.cpp | 39 +++++++++++++++++++++++++--- trunk/src/app/srs_app_rtc_conn.hpp | 3 +++ trunk/src/core/srs_core_version6.hpp | 2 +- 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/trunk/doc/CHANGELOG.md b/trunk/doc/CHANGELOG.md index b4024b35f..0b21f0f32 100644 --- a/trunk/doc/CHANGELOG.md +++ b/trunk/doc/CHANGELOG.md @@ -7,6 +7,7 @@ The changelog for SRS. ## SRS 6.0 Changelog +* v6.0, 2025-09-05, RTX: Fix race condition for timer. v6.0.176 (#4470) (#4474) * v6.0, 2025-08-26, Merge [#4451](https://github.com/ossrs/srs/pull/4451): RTC: Fix null pointer crash in RTC2RTMP when start packet is missing. v6.0.175 (#4451) * v6.0, 2025-08-16, Merge [#4441](https://github.com/ossrs/srs/pull/4441): fix err memory leak in rtc to rtmp bridge. v6.0.174 (#4441) * v6.0, 2025-08-14, Merge [#4161](https://github.com/ossrs/srs/pull/4161): fix hls & dash segments cleanup. v6.0.173 (#4161) diff --git a/trunk/src/app/srs_app_rtc_conn.cpp b/trunk/src/app/srs_app_rtc_conn.cpp index af2fbcfdc..bd324d7a3 100644 --- a/trunk/src/app/srs_app_rtc_conn.cpp +++ b/trunk/src/app/srs_app_rtc_conn.cpp @@ -945,18 +945,29 @@ srs_error_t SrsRtcPlayStream::do_request_keyframe(uint32_t ssrc, SrsContextId ci SrsRtcPublishRtcpTimer::SrsRtcPublishRtcpTimer(SrsRtcPublishStream* p) : p_(p) { + lock_ = srs_mutex_new(); _srs_hybrid->timer1s()->subscribe(this); } SrsRtcPublishRtcpTimer::~SrsRtcPublishRtcpTimer() { - _srs_hybrid->timer1s()->unsubscribe(this); + if (true) { + SrsLocker(lock_); + _srs_hybrid->timer1s()->unsubscribe(this); + } + srs_mutex_destroy(lock_); } srs_error_t SrsRtcPublishRtcpTimer::on_timer(srs_utime_t interval) { srs_error_t err = srs_success; + // This is a very heavy function, and it may potentially cause a coroutine switch. + // Therefore, during this function, the 'this' pointer might become invalid because + // the object could be freed by another thread. As a result, we must lock the object + // to prevent it from being freed. + SrsLocker(lock_); + ++_srs_pps_pub->sugar; if (!p_->is_started) { @@ -981,18 +992,29 @@ srs_error_t SrsRtcPublishRtcpTimer::on_timer(srs_utime_t interval) SrsRtcPublishTwccTimer::SrsRtcPublishTwccTimer(SrsRtcPublishStream* p) : p_(p) { + lock_ = srs_mutex_new(); _srs_hybrid->timer100ms()->subscribe(this); } SrsRtcPublishTwccTimer::~SrsRtcPublishTwccTimer() { - _srs_hybrid->timer100ms()->unsubscribe(this); + if (true) { + SrsLocker(lock_); + _srs_hybrid->timer100ms()->unsubscribe(this); + } + srs_mutex_destroy(lock_); } srs_error_t SrsRtcPublishTwccTimer::on_timer(srs_utime_t interval) { srs_error_t err = srs_success; + // This is a very heavy function, and it may potentially cause a coroutine switch. + // Therefore, during this function, the 'this' pointer might become invalid because + // the object could be freed by another thread. As a result, we must lock the object + // to prevent it from being freed. + SrsLocker(lock_); + ++_srs_pps_pub->sugar; if (!p_->is_started) { @@ -1739,12 +1761,17 @@ void SrsRtcPublishStream::update_send_report_time(uint32_t ssrc, const SrsNtp& n SrsRtcConnectionNackTimer::SrsRtcConnectionNackTimer(SrsRtcConnection* p) : p_(p) { + lock_ = srs_mutex_new(); _srs_hybrid->timer20ms()->subscribe(this); } SrsRtcConnectionNackTimer::~SrsRtcConnectionNackTimer() { - _srs_hybrid->timer20ms()->unsubscribe(this); + if (true) { + SrsLocker(lock_); + _srs_hybrid->timer20ms()->unsubscribe(this); + } + srs_mutex_destroy(lock_); } srs_error_t SrsRtcConnectionNackTimer::on_timer(srs_utime_t interval) @@ -1755,6 +1782,12 @@ srs_error_t SrsRtcConnectionNackTimer::on_timer(srs_utime_t interval) return err; } + // This is a very heavy function, and it may potentially cause a coroutine switch. + // Therefore, during this function, the 'this' pointer might become invalid because + // the object could be freed by another thread. As a result, we must lock the object + // to prevent it from being freed. + SrsLocker(lock_); + ++_srs_pps_conn->sugar; // If circuit-breaker is enabled, disable nack. diff --git a/trunk/src/app/srs_app_rtc_conn.hpp b/trunk/src/app/srs_app_rtc_conn.hpp index 5a2df2700..4275a6792 100644 --- a/trunk/src/app/srs_app_rtc_conn.hpp +++ b/trunk/src/app/srs_app_rtc_conn.hpp @@ -283,6 +283,7 @@ class SrsRtcPublishRtcpTimer : public ISrsFastTimer { private: SrsRtcPublishStream* p_; + srs_mutex_t lock_; public: SrsRtcPublishRtcpTimer(SrsRtcPublishStream* p); virtual ~SrsRtcPublishRtcpTimer(); @@ -296,6 +297,7 @@ class SrsRtcPublishTwccTimer : public ISrsFastTimer { private: SrsRtcPublishStream* p_; + srs_mutex_t lock_; public: SrsRtcPublishTwccTimer(SrsRtcPublishStream* p); virtual ~SrsRtcPublishTwccTimer(); @@ -406,6 +408,7 @@ class SrsRtcConnectionNackTimer : public ISrsFastTimer { private: SrsRtcConnection* p_; + srs_mutex_t lock_; public: SrsRtcConnectionNackTimer(SrsRtcConnection* p); virtual ~SrsRtcConnectionNackTimer(); diff --git a/trunk/src/core/srs_core_version6.hpp b/trunk/src/core/srs_core_version6.hpp index 37016a7b0..9ff033141 100644 --- a/trunk/src/core/srs_core_version6.hpp +++ b/trunk/src/core/srs_core_version6.hpp @@ -9,6 +9,6 @@ #define VERSION_MAJOR 6 #define VERSION_MINOR 0 -#define VERSION_REVISION 175 +#define VERSION_REVISION 176 #endif