WebRTC: Fix race condition in RTC publish timer callbacks. v7.0.76 (#4470)

WebRTC RTC publish streams use timer callbacks (`SrsRtcPublishRtcpTimer`
and `SrsRtcPublishTwccTimer`) that can cause race conditions in SRS's
coroutine-based architecture. The timer callbacks are heavy functions
that may trigger coroutine switches, during which the timer object can
be freed by another coroutine, leading to use-after-free crashes.

The race condition occurs because:
1. Timer callbacks (`on_timer`) perform heavy operations that can yield
control
2. During coroutine switches, other coroutines may destroy the timer
object
3. When control returns, the callback continues executing on a freed
object

Fixes potential crashes in WebRTC RTC publish streams under high
concurrency.
This commit is contained in:
Winlin 2025-09-03 19:45:24 -04:00 committed by GitHub
parent d9fe2c458c
commit 421ab6c3fb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 70 additions and 45 deletions

View File

@ -7,6 +7,7 @@ The changelog for SRS.
<a name="v7-changes"></a>
## SRS 7.0 Changelog
* v7.0, 2025-09-03, Merge [#4470](https://github.com/ossrs/srs/pull/4470): RTX: Fix race condition for timer. v7.0.76 (#4470)
* v7.0, 2025-09-02, Merge [#4466](https://github.com/ossrs/srs/pull/4466): AI: GB28181: Remove embedded SIP server and enforce external SIP usage. v7.0.75 (#4466)
* v7.0, 2025-09-01, Merge [#4465](https://github.com/ossrs/srs/pull/4465): AI: Replace SrsSharedPtrMessage with SrsMediaPacket for unified media packet handling. v7.0.74 (#4465)
* v7.0, 2025-09-01, Merge [#4464](https://github.com/ossrs/srs/pull/4464): AI: Use shared ptr in RTMP message. v7.0.73 (#4464)

View File

@ -928,18 +928,29 @@ srs_error_t SrsRtcPlayStream::do_request_keyframe(uint32_t ssrc, SrsContextId ci
SrsRtcPublishRtcpTimer::SrsRtcPublishRtcpTimer(SrsRtcPublishStream *p) : p_(p)
{
lock_ = srs_mutex_new();
_srs_shared_timer->timer1s()->subscribe(this);
}
SrsRtcPublishRtcpTimer::~SrsRtcPublishRtcpTimer()
{
_srs_shared_timer->timer1s()->unsubscribe(this);
if (true) {
SrsLocker(lock_);
_srs_shared_timer->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) {
@ -964,18 +975,29 @@ srs_error_t SrsRtcPublishRtcpTimer::on_timer(srs_utime_t interval)
SrsRtcPublishTwccTimer::SrsRtcPublishTwccTimer(SrsRtcPublishStream *p) : p_(p)
{
lock_ = srs_mutex_new();
_srs_shared_timer->timer100ms()->subscribe(this);
}
SrsRtcPublishTwccTimer::~SrsRtcPublishTwccTimer()
{
_srs_shared_timer->timer100ms()->unsubscribe(this);
if (true) {
SrsLocker(lock_);
_srs_shared_timer->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) {

View File

@ -306,6 +306,7 @@ class SrsRtcPublishRtcpTimer : public ISrsFastTimer
{
private:
SrsRtcPublishStream *p_;
srs_mutex_t lock_;
public:
SrsRtcPublishRtcpTimer(SrsRtcPublishStream *p);
@ -320,6 +321,7 @@ class SrsRtcPublishTwccTimer : public ISrsFastTimer
{
private:
SrsRtcPublishStream *p_;
srs_mutex_t lock_;
public:
SrsRtcPublishTwccTimer(SrsRtcPublishStream *p);

View File

@ -9,6 +9,6 @@
#define VERSION_MAJOR 7
#define VERSION_MINOR 0
#define VERSION_REVISION 75
#define VERSION_REVISION 76
#endif

View File

@ -863,48 +863,48 @@ enum http_status {
};
/* Request Methods */
#define HTTP_METHOD_MAP(XX) \
XX(0, DELETE, DELETE) \
XX(1, GET, GET) \
XX(2, HEAD, HEAD) \
XX(3, POST, POST) \
XX(4, PUT, PUT) \
/* pathological */ \
XX(5, CONNECT, CONNECT) \
XX(6, OPTIONS, OPTIONS) \
XX(7, TRACE, TRACE) \
/* WebDAV */ \
XX(8, COPY, COPY) \
XX(9, LOCK, LOCK) \
XX(10, MKCOL, MKCOL) \
XX(11, MOVE, MOVE) \
XX(12, PROPFIND, PROPFIND) \
XX(13, PROPPATCH, PROPPATCH) \
XX(14, SEARCH, SEARCH) \
XX(15, UNLOCK, UNLOCK) \
XX(16, BIND, BIND) \
XX(17, REBIND, REBIND) \
XX(18, UNBIND, UNBIND) \
XX(19, ACL, ACL) \
/* subversion */ \
XX(20, REPORT, REPORT) \
XX(21, MKACTIVITY, MKACTIVITY) \
XX(22, CHECKOUT, CHECKOUT) \
XX(23, MERGE, MERGE) \
/* upnp */ \
XX(24, MSEARCH, M - SEARCH) \
XX(25, NOTIFY, NOTIFY) \
XX(26, SUBSCRIBE, SUBSCRIBE) \
XX(27, UNSUBSCRIBE, UNSUBSCRIBE) \
/* RFC-5789 */ \
XX(28, PATCH, PATCH) \
XX(29, PURGE, PURGE) \
/* CalDAV */ \
XX(30, MKCALENDAR, MKCALENDAR) \
/* RFC-2068, section 19.6.1.2 */ \
XX(31, LINK, LINK) \
XX(32, UNLINK, UNLINK) \
/* icecast */ \
#define HTTP_METHOD_MAP(XX) \
XX(0, DELETE, DELETE) \
XX(1, GET, GET) \
XX(2, HEAD, HEAD) \
XX(3, POST, POST) \
XX(4, PUT, PUT) \
/* pathological */ \
XX(5, CONNECT, CONNECT) \
XX(6, OPTIONS, OPTIONS) \
XX(7, TRACE, TRACE) \
/* WebDAV */ \
XX(8, COPY, COPY) \
XX(9, LOCK, LOCK) \
XX(10, MKCOL, MKCOL) \
XX(11, MOVE, MOVE) \
XX(12, PROPFIND, PROPFIND) \
XX(13, PROPPATCH, PROPPATCH) \
XX(14, SEARCH, SEARCH) \
XX(15, UNLOCK, UNLOCK) \
XX(16, BIND, BIND) \
XX(17, REBIND, REBIND) \
XX(18, UNBIND, UNBIND) \
XX(19, ACL, ACL) \
/* subversion */ \
XX(20, REPORT, REPORT) \
XX(21, MKACTIVITY, MKACTIVITY) \
XX(22, CHECKOUT, CHECKOUT) \
XX(23, MERGE, MERGE) \
/* upnp */ \
XX(24, MSEARCH, M - SEARCH) \
XX(25, NOTIFY, NOTIFY) \
XX(26, SUBSCRIBE, SUBSCRIBE) \
XX(27, UNSUBSCRIBE, UNSUBSCRIBE) \
/* RFC-5789 */ \
XX(28, PATCH, PATCH) \
XX(29, PURGE, PURGE) \
/* CalDAV */ \
XX(30, MKCALENDAR, MKCALENDAR) \
/* RFC-2068, section 19.6.1.2 */ \
XX(31, LINK, LINK) \
XX(32, UNLINK, UNLINK) \
/* icecast */ \
XX(33, SOURCE, SOURCE)
enum http_method {