HTTP-FLV: Crash when multiple viewers. v6.0.148 v7.0.5 (#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 <liumengte@visionular.com>
Co-authored-by: winlin <winlinvip@gmail.com>
pull/3987/merge
Bahamut 6 months ago committed by winlin
parent e323215478
commit 3e811ba34a

@ -7,6 +7,7 @@ The changelog for SRS.
<a name="v7-changes"></a> <a name="v7-changes"></a>
## SRS 7.0 Changelog ## SRS 7.0 Changelog
* v7.0, 2024-08-15, Merge [#4144](https://github.com/ossrs/srs/pull/4144): HTTP-FLV: Crash when multiple viewers. v7.0.5 (#4144)
* v7.0, 2024-08-15, Merge [#4142](https://github.com/ossrs/srs/pull/4142): Config: Add more utest for env config. v7.0.4 (#4142) * v7.0, 2024-08-15, Merge [#4142](https://github.com/ossrs/srs/pull/4142): Config: Add more utest for env config. v7.0.4 (#4142)
* v7.0, 2024-08-14, Merge [#4141](https://github.com/ossrs/srs/pull/4141): Live: Crash for invalid live stream state when unmount HTTP. v7.0.3 (#4141) * v7.0, 2024-08-14, Merge [#4141](https://github.com/ossrs/srs/pull/4141): Live: Crash for invalid live stream state when unmount HTTP. v7.0.3 (#4141)
* v7.0, 2024-08-13, Merge [#4092](https://github.com/ossrs/srs/pull/4092): Config: Improve env config to support multi values. v7.0.2 (#4092) * v7.0, 2024-08-13, Merge [#4092](https://github.com/ossrs/srs/pull/4092): Config: Improve env config to support multi values. v7.0.2 (#4092)
@ -16,6 +17,7 @@ The changelog for SRS.
<a name="v6-changes"></a> <a name="v6-changes"></a>
## SRS 6.0 Changelog ## SRS 6.0 Changelog
* v6.0, 2024-08-15, Merge [#4144](https://github.com/ossrs/srs/pull/4144): HTTP-FLV: Crash when multiple viewers. v6.0.148 (#4144)
* 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) * 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)
* 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) * 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)
* v6.0, 2024-07-27, Merge [#4127](https://github.com/ossrs/srs/pull/4127): Transcode: Support video codec such as h264_qsv and libx265. v6.0.145 (#4127) * v6.0, 2024-07-27, Merge [#4127](https://github.com/ossrs/srs/pull/4127): Transcode: Support video codec such as h264_qsv and libx265. v6.0.145 (#4127)

@ -583,7 +583,7 @@ SrsLiveStream::SrsLiveStream(SrsRequest* r, SrsBufferCache* c)
cache = c; cache = c;
req = r->copy()->as_http(); req = r->copy()->as_http();
security_ = new SrsSecurity(); security_ = new SrsSecurity();
alive_ = false; alive_viewers_ = 0;
} }
SrsLiveStream::~SrsLiveStream() SrsLiveStream::~SrsLiveStream()
@ -634,9 +634,9 @@ srs_error_t SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage
return srs_error_wrap(err, "http hook"); return srs_error_wrap(err, "http hook");
} }
alive_ = true; alive_viewers_++;
err = do_serve_http(w, r); err = do_serve_http(w, r);
alive_ = false; alive_viewers_--;
http_hooks_on_stop(r); http_hooks_on_stop(r);
@ -645,7 +645,7 @@ srs_error_t SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage
bool SrsLiveStream::alive() bool SrsLiveStream::alive()
{ {
return alive_; return alive_viewers_ > 0;
} }
srs_error_t SrsLiveStream::do_serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage* r) srs_error_t SrsLiveStream::do_serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage* r)

@ -182,7 +182,10 @@ private:
SrsRequest* req; SrsRequest* req;
SrsBufferCache* cache; SrsBufferCache* cache;
SrsSecurity* security_; SrsSecurity* security_;
bool alive_; // 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_viewers_;
public: public:
SrsLiveStream(SrsRequest* r, SrsBufferCache* c); SrsLiveStream(SrsRequest* r, SrsBufferCache* c);
virtual ~SrsLiveStream(); virtual ~SrsLiveStream();

@ -1886,7 +1886,7 @@ SrsLiveSource::SrsLiveSource()
mix_correct = false; mix_correct = false;
mix_queue = new SrsMixQueue(); mix_queue = new SrsMixQueue();
_can_publish = true; can_publish_ = true;
stream_die_at_ = 0; stream_die_at_ = 0;
publisher_idle_at_ = 0; publisher_idle_at_ = 0;
@ -1952,7 +1952,7 @@ srs_error_t SrsLiveSource::cycle()
bool SrsLiveSource::stream_is_dead() bool SrsLiveSource::stream_is_dead()
{ {
// still publishing? // still publishing?
if (!_can_publish || !publish_edge->can_publish()) { if (!can_publish_ || !publish_edge->can_publish()) {
return false; return false;
} }
@ -2151,7 +2151,7 @@ SrsContextId SrsLiveSource::pre_source_id()
bool SrsLiveSource::inactive() bool SrsLiveSource::inactive()
{ {
return _can_publish; return can_publish_;
} }
void SrsLiveSource::update_auth(SrsRequest* r) void SrsLiveSource::update_auth(SrsRequest* r)
@ -2167,7 +2167,7 @@ bool SrsLiveSource::can_publish(bool is_edge)
return publish_edge->can_publish(); return publish_edge->can_publish();
} }
return _can_publish; return can_publish_;
} }
srs_error_t SrsLiveSource::on_meta_data(SrsCommonMessage* msg, SrsOnMetaDataPacket* metadata) 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. // update the request object.
srs_assert(req); srs_assert(req);
_can_publish = false; can_publish_ = false;
// whatever, the publish thread is the source or edge source, // whatever, the publish thread is the source or edge source,
// save its id to srouce id. // save its id to srouce id.
@ -2614,7 +2614,7 @@ srs_error_t SrsLiveSource::on_publish()
void SrsLiveSource::on_unpublish() void SrsLiveSource::on_unpublish()
{ {
// ignore when already unpublished. // ignore when already unpublished.
if (_can_publish) { if (can_publish_) {
return; return;
} }
@ -2655,7 +2655,10 @@ void SrsLiveSource::on_unpublish()
stream_die_at_ = srs_get_system_time(); stream_die_at_ = srs_get_system_time();
} }
_can_publish = true; // 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;
} }
srs_error_t SrsLiveSource::create_consumer(SrsLiveConsumer*& consumer) srs_error_t SrsLiveSource::create_consumer(SrsLiveConsumer*& consumer)
@ -2735,7 +2738,7 @@ void SrsLiveSource::on_consumer_destroy(SrsLiveConsumer* consumer)
play_edge->on_all_client_stop(); play_edge->on_all_client_stop();
// If no publishers, the stream is die. // If no publishers, the stream is die.
if (_can_publish) { if (can_publish_) {
stream_die_at_ = srs_get_system_time(); stream_die_at_ = srs_get_system_time();
} }

@ -529,7 +529,7 @@ private:
SrsRtmpFormat* format_; SrsRtmpFormat* format_;
private: private:
// Whether source is avaiable for publishing. // Whether source is avaiable for publishing.
bool _can_publish; bool can_publish_;
// The last die time, while die means neither publishers nor players. // The last die time, while die means neither publishers nor players.
srs_utime_t stream_die_at_; srs_utime_t stream_die_at_;
// The last idle time, while idle means no players. // The last idle time, while idle means no players.

@ -9,6 +9,6 @@
#define VERSION_MAJOR 6 #define VERSION_MAJOR 6
#define VERSION_MINOR 0 #define VERSION_MINOR 0
#define VERSION_REVISION 147 #define VERSION_REVISION 148
#endif #endif

@ -9,6 +9,6 @@
#define VERSION_MAJOR 7 #define VERSION_MAJOR 7
#define VERSION_MINOR 0 #define VERSION_MINOR 0
#define VERSION_REVISION 4 #define VERSION_REVISION 5
#endif #endif
Loading…
Cancel
Save