Skip to content

Commit 3917422

Browse files
retamialiumengtewinlinvip
committed
HTTP-FLV: Crash when multiple viewers. v6.0.148 (#4144)
I did some preliminary code inspection. The two playback endpoints share the same `SrsLiveStream` instance. After the first one disconnects, `alive_` is set to false. ``` alive_ = true; err = do_serve_http(w, r); alive_ = false; ``` In the `SrsHttpStreamServer::http_unmount(SrsRequest* r)` function, `stream->alive()` is already false, so `mux.unhandle` will free the `SrsLiveStream`. This causes the other connection coroutine to return to its execution environment after the `SrsLiveStream` instance has already been freed. ``` // Wait for cache and stream to stop. int i = 0; for (; i < 1024; i++) { if (!cache->alive() && !stream->alive()) { break; } srs_usleep(100 * SRS_UTIME_MILLISECONDS); } // Unmount the HTTP handler, which will free the entry. Note that we must free it after cache and // stream stopped for it uses it. mux.unhandle(entry->mount, stream.get()); ``` `alive_` was changed from a `bool` to an `int` to ensure that `mux.unhandle` is only executed after each connection's `serve_http` has exited. --------- Co-authored-by: liumengte <[email protected]> Co-authored-by: winlin <[email protected]>
1 parent 133a39a commit 3917422

6 files changed

+22
-15
lines changed

trunk/doc/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ The changelog for SRS.
77
<a name="v6-changes"></a>
88

99
## SRS 6.0 Changelog
10+
* v6.0, 2024-08-15, Merge [#4144](https://github.com/ossrs/srs/pull/4144): HTTP-FLV: Crash when multiple viewers. v6.0.148 (#4144)
1011
* v6.0, 2024-08-15, Merge [#4142](https://github.com/ossrs/srs/pull/4142): Config: Add more utest for env config. v6.0.147 (#4142)
1112
* v6.0, 2024-08-14, Merge [#4141](https://github.com/ossrs/srs/pull/4141): Live: Crash for invalid live stream state when unmount HTTP. v6.0.146 (#4141)
1213
* v6.0, 2024-08-13, Merge [#4092](https://github.com/ossrs/srs/pull/4092): Config: Improve env config to support multi values. v6.0.146 (#4092)

trunk/src/app/srs_app_http_stream.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ SrsLiveStream::SrsLiveStream(SrsRequest* r, SrsBufferCache* c)
583583
cache = c;
584584
req = r->copy()->as_http();
585585
security_ = new SrsSecurity();
586-
alive_ = false;
586+
alive_viewers_ = 0;
587587
}
588588

589589
SrsLiveStream::~SrsLiveStream()
@@ -634,9 +634,9 @@ srs_error_t SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage
634634
return srs_error_wrap(err, "http hook");
635635
}
636636

637-
alive_ = true;
637+
alive_viewers_++;
638638
err = do_serve_http(w, r);
639-
alive_ = false;
639+
alive_viewers_--;
640640

641641
http_hooks_on_stop(r);
642642

@@ -645,7 +645,7 @@ srs_error_t SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage
645645

646646
bool SrsLiveStream::alive()
647647
{
648-
return alive_;
648+
return alive_viewers_ > 0;
649649
}
650650

651651
srs_error_t SrsLiveStream::do_serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage* r)

trunk/src/app/srs_app_http_stream.hpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,10 @@ class SrsLiveStream : public ISrsHttpHandler
182182
SrsRequest* req;
183183
SrsBufferCache* cache;
184184
SrsSecurity* security_;
185-
bool alive_;
185+
// For multiple viewers, which means there will more than one alive viewers for a live stream, so we must
186+
// use an int value to represent if there is any viewer is alive. We should never do cleanup unless all
187+
// viewers closed the connection.
188+
int alive_viewers_;
186189
public:
187190
SrsLiveStream(SrsRequest* r, SrsBufferCache* c);
188191
virtual ~SrsLiveStream();

trunk/src/app/srs_app_source.cpp

+11-8
Original file line numberDiff line numberDiff line change
@@ -1886,7 +1886,7 @@ SrsLiveSource::SrsLiveSource()
18861886
mix_correct = false;
18871887
mix_queue = new SrsMixQueue();
18881888

1889-
_can_publish = true;
1889+
can_publish_ = true;
18901890
stream_die_at_ = 0;
18911891
publisher_idle_at_ = 0;
18921892

@@ -1952,7 +1952,7 @@ srs_error_t SrsLiveSource::cycle()
19521952
bool SrsLiveSource::stream_is_dead()
19531953
{
19541954
// still publishing?
1955-
if (!_can_publish || !publish_edge->can_publish()) {
1955+
if (!can_publish_ || !publish_edge->can_publish()) {
19561956
return false;
19571957
}
19581958

@@ -2151,7 +2151,7 @@ SrsContextId SrsLiveSource::pre_source_id()
21512151

21522152
bool SrsLiveSource::inactive()
21532153
{
2154-
return _can_publish;
2154+
return can_publish_;
21552155
}
21562156

21572157
void SrsLiveSource::update_auth(SrsRequest* r)
@@ -2167,7 +2167,7 @@ bool SrsLiveSource::can_publish(bool is_edge)
21672167
return publish_edge->can_publish();
21682168
}
21692169

2170-
return _can_publish;
2170+
return can_publish_;
21712171
}
21722172

21732173
srs_error_t SrsLiveSource::on_meta_data(SrsCommonMessage* msg, SrsOnMetaDataPacket* metadata)
@@ -2566,7 +2566,7 @@ srs_error_t SrsLiveSource::on_publish()
25662566
// update the request object.
25672567
srs_assert(req);
25682568

2569-
_can_publish = false;
2569+
can_publish_ = false;
25702570

25712571
// whatever, the publish thread is the source or edge source,
25722572
// save its id to srouce id.
@@ -2614,7 +2614,7 @@ srs_error_t SrsLiveSource::on_publish()
26142614
void SrsLiveSource::on_unpublish()
26152615
{
26162616
// ignore when already unpublished.
2617-
if (_can_publish) {
2617+
if (can_publish_) {
26182618
return;
26192619
}
26202620

@@ -2655,7 +2655,10 @@ void SrsLiveSource::on_unpublish()
26552655
stream_die_at_ = srs_get_system_time();
26562656
}
26572657

2658-
_can_publish = true;
2658+
// Note that we should never set to unpublish before any other handler is done, especially the handler
2659+
// which is actually an http stream that unmounts the HTTP path for streaming, because there maybe some
2660+
// coroutine switch in these handlers.
2661+
can_publish_ = true;
26592662
}
26602663

26612664
srs_error_t SrsLiveSource::create_consumer(SrsLiveConsumer*& consumer)
@@ -2735,7 +2738,7 @@ void SrsLiveSource::on_consumer_destroy(SrsLiveConsumer* consumer)
27352738
play_edge->on_all_client_stop();
27362739

27372740
// If no publishers, the stream is die.
2738-
if (_can_publish) {
2741+
if (can_publish_) {
27392742
stream_die_at_ = srs_get_system_time();
27402743
}
27412744

trunk/src/app/srs_app_source.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ class SrsLiveSource : public ISrsReloadHandler
529529
SrsRtmpFormat* format_;
530530
private:
531531
// Whether source is avaiable for publishing.
532-
bool _can_publish;
532+
bool can_publish_;
533533
// The last die time, while die means neither publishers nor players.
534534
srs_utime_t stream_die_at_;
535535
// The last idle time, while idle means no players.

trunk/src/core/srs_core_version6.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@
99

1010
#define VERSION_MAJOR 6
1111
#define VERSION_MINOR 0
12-
#define VERSION_REVISION 147
12+
#define VERSION_REVISION 148
1313

1414
#endif

0 commit comments

Comments
 (0)