From c3b6d971d1e358cf724918acb5c0cb82cb5a775a Mon Sep 17 00:00:00 2001 From: liumengte Date: Wed, 14 Aug 2024 10:25:31 +0800 Subject: [PATCH 1/5] fix: crash on unpublish when multiple viewers are connected https://github.com/ossrs/srs/issues/4143 --- trunk/src/app/srs_app_http_stream.cpp | 8 ++++---- trunk/src/app/srs_app_http_stream.hpp | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/trunk/src/app/srs_app_http_stream.cpp b/trunk/src/app/srs_app_http_stream.cpp index 2a97f96163..8a932bc89b 100755 --- a/trunk/src/app/srs_app_http_stream.cpp +++ b/trunk/src/app/srs_app_http_stream.cpp @@ -583,7 +583,7 @@ SrsLiveStream::SrsLiveStream(SrsRequest* r, SrsBufferCache* c) cache = c; req = r->copy()->as_http(); security_ = new SrsSecurity(); - alive_ = false; + alive_ = 0; } SrsLiveStream::~SrsLiveStream() @@ -634,9 +634,9 @@ srs_error_t SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage return srs_error_wrap(err, "http hook"); } - alive_ = true; + alive_++; err = do_serve_http(w, r); - alive_ = false; + alive_--; http_hooks_on_stop(r); @@ -645,7 +645,7 @@ srs_error_t SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage bool SrsLiveStream::alive() { - return alive_; + return alive_ > 0; } srs_error_t SrsLiveStream::do_serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage* r) diff --git a/trunk/src/app/srs_app_http_stream.hpp b/trunk/src/app/srs_app_http_stream.hpp index 64e826c014..8d962b151e 100755 --- a/trunk/src/app/srs_app_http_stream.hpp +++ b/trunk/src/app/srs_app_http_stream.hpp @@ -182,7 +182,7 @@ class SrsLiveStream : public ISrsHttpHandler SrsRequest* req; SrsBufferCache* cache; SrsSecurity* security_; - bool alive_; + int alive_; public: SrsLiveStream(SrsRequest* r, SrsBufferCache* c); virtual ~SrsLiveStream(); From eed028105ae2fc924597fec67883b874ce2f8582 Mon Sep 17 00:00:00 2001 From: winlin Date: Thu, 15 Aug 2024 11:21:46 +0800 Subject: [PATCH 2/5] Refine comments. --- trunk/src/app/srs_app_http_stream.hpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/trunk/src/app/srs_app_http_stream.hpp b/trunk/src/app/srs_app_http_stream.hpp index 8d962b151e..8abd28313a 100755 --- a/trunk/src/app/srs_app_http_stream.hpp +++ b/trunk/src/app/srs_app_http_stream.hpp @@ -182,6 +182,9 @@ class SrsLiveStream : public ISrsHttpHandler SrsRequest* req; SrsBufferCache* cache; SrsSecurity* security_; + // For multiple viewers, which means there will more than one alive viewers for a live stream, so we must + // use an int value to represent if there is any viewer is alive. We should never do cleanup unless all + // viewers closed the connection. int alive_; public: SrsLiveStream(SrsRequest* r, SrsBufferCache* c); From 30f2f8633b01397233e2fede1351bf3fc9eaa96d Mon Sep 17 00:00:00 2001 From: winlin Date: Thu, 15 Aug 2024 11:24:05 +0800 Subject: [PATCH 3/5] Refine comments. --- trunk/src/app/srs_app_source.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/trunk/src/app/srs_app_source.cpp b/trunk/src/app/srs_app_source.cpp index bdbfc829e9..cca7884bd9 100755 --- a/trunk/src/app/srs_app_source.cpp +++ b/trunk/src/app/srs_app_source.cpp @@ -2655,6 +2655,9 @@ void SrsLiveSource::on_unpublish() stream_die_at_ = srs_get_system_time(); } + // Note that we should never set to unpublish before any other handler is done, especially the handler + // which is actually an http stream that unmounts the HTTP path for streaming, because there maybe some + // coroutine switch in these handlers. _can_publish = true; } From b4478eab982cb8f5f0bc9d233dde89dfed2dd398 Mon Sep 17 00:00:00 2001 From: winlin Date: Thu, 15 Aug 2024 11:24:42 +0800 Subject: [PATCH 4/5] Only rename member field. --- trunk/src/app/srs_app_source.cpp | 16 ++++++++-------- trunk/src/app/srs_app_source.hpp | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/trunk/src/app/srs_app_source.cpp b/trunk/src/app/srs_app_source.cpp index cca7884bd9..956f7b2e3e 100755 --- a/trunk/src/app/srs_app_source.cpp +++ b/trunk/src/app/srs_app_source.cpp @@ -1886,7 +1886,7 @@ SrsLiveSource::SrsLiveSource() mix_correct = false; mix_queue = new SrsMixQueue(); - _can_publish = true; + can_publish_ = true; stream_die_at_ = 0; publisher_idle_at_ = 0; @@ -1952,7 +1952,7 @@ srs_error_t SrsLiveSource::cycle() bool SrsLiveSource::stream_is_dead() { // still publishing? - if (!_can_publish || !publish_edge->can_publish()) { + if (!can_publish_ || !publish_edge->can_publish()) { return false; } @@ -2151,7 +2151,7 @@ SrsContextId SrsLiveSource::pre_source_id() bool SrsLiveSource::inactive() { - return _can_publish; + return can_publish_; } void SrsLiveSource::update_auth(SrsRequest* r) @@ -2167,7 +2167,7 @@ bool SrsLiveSource::can_publish(bool is_edge) return publish_edge->can_publish(); } - return _can_publish; + return can_publish_; } srs_error_t SrsLiveSource::on_meta_data(SrsCommonMessage* msg, SrsOnMetaDataPacket* metadata) @@ -2566,7 +2566,7 @@ srs_error_t SrsLiveSource::on_publish() // update the request object. srs_assert(req); - _can_publish = false; + can_publish_ = false; // whatever, the publish thread is the source or edge source, // save its id to srouce id. @@ -2614,7 +2614,7 @@ srs_error_t SrsLiveSource::on_publish() void SrsLiveSource::on_unpublish() { // ignore when already unpublished. - if (_can_publish) { + if (can_publish_) { return; } @@ -2658,7 +2658,7 @@ void SrsLiveSource::on_unpublish() // Note that we should never set to unpublish before any other handler is done, especially the handler // which is actually an http stream that unmounts the HTTP path for streaming, because there maybe some // coroutine switch in these handlers. - _can_publish = true; + can_publish_ = true; } srs_error_t SrsLiveSource::create_consumer(SrsLiveConsumer*& consumer) @@ -2738,7 +2738,7 @@ void SrsLiveSource::on_consumer_destroy(SrsLiveConsumer* consumer) play_edge->on_all_client_stop(); // If no publishers, the stream is die. - if (_can_publish) { + if (can_publish_) { stream_die_at_ = srs_get_system_time(); } diff --git a/trunk/src/app/srs_app_source.hpp b/trunk/src/app/srs_app_source.hpp index 67d0a48fac..e10b441de9 100644 --- a/trunk/src/app/srs_app_source.hpp +++ b/trunk/src/app/srs_app_source.hpp @@ -529,7 +529,7 @@ class SrsLiveSource : public ISrsReloadHandler SrsRtmpFormat* format_; private: // Whether source is avaiable for publishing. - bool _can_publish; + bool can_publish_; // The last die time, while die means neither publishers nor players. srs_utime_t stream_die_at_; // The last idle time, while idle means no players. From 4376b788cb14e6fcd2f7136ed1351ca02843a197 Mon Sep 17 00:00:00 2001 From: winlin Date: Thu, 15 Aug 2024 11:26:18 +0800 Subject: [PATCH 5/5] Rename memember field. --- trunk/src/app/srs_app_http_stream.cpp | 8 ++++---- trunk/src/app/srs_app_http_stream.hpp | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/trunk/src/app/srs_app_http_stream.cpp b/trunk/src/app/srs_app_http_stream.cpp index 8a932bc89b..b6f7fa47ba 100755 --- a/trunk/src/app/srs_app_http_stream.cpp +++ b/trunk/src/app/srs_app_http_stream.cpp @@ -583,7 +583,7 @@ SrsLiveStream::SrsLiveStream(SrsRequest* r, SrsBufferCache* c) cache = c; req = r->copy()->as_http(); security_ = new SrsSecurity(); - alive_ = 0; + alive_viewers_ = 0; } SrsLiveStream::~SrsLiveStream() @@ -634,9 +634,9 @@ srs_error_t SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage return srs_error_wrap(err, "http hook"); } - alive_++; + alive_viewers_++; err = do_serve_http(w, r); - alive_--; + alive_viewers_--; http_hooks_on_stop(r); @@ -645,7 +645,7 @@ srs_error_t SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage bool SrsLiveStream::alive() { - return alive_ > 0; + return alive_viewers_ > 0; } srs_error_t SrsLiveStream::do_serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage* r) diff --git a/trunk/src/app/srs_app_http_stream.hpp b/trunk/src/app/srs_app_http_stream.hpp index 8abd28313a..2c557972ba 100755 --- a/trunk/src/app/srs_app_http_stream.hpp +++ b/trunk/src/app/srs_app_http_stream.hpp @@ -185,7 +185,7 @@ class SrsLiveStream : public ISrsHttpHandler // For multiple viewers, which means there will more than one alive viewers for a live stream, so we must // use an int value to represent if there is any viewer is alive. We should never do cleanup unless all // viewers closed the connection. - int alive_; + int alive_viewers_; public: SrsLiveStream(SrsRequest* r, SrsBufferCache* c); virtual ~SrsLiveStream();