SRS5: API: Fix duplicated on_stop callback event bug. v5.0.125 (ossrs…

…#3349) * fix hls bug:Duplicated on_stop callback * improve utest * Refine magic number. * API: Fix duplicated on_stop callback event bug. v5.0.125 Co-authored-by: winlin <[email protected]> PICK 3727d05
bahari · Jan 1, 2023 · 57cc843 · 57cc843
1 parent e4e87c0
commit 57cc843
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 12 deletions.
diff --git a/trunk/doc/CHANGELOG.md b/trunk/doc/CHANGELOG.md
@@ -24,6 +24,7 @@ The changelog for SRS.
 
 ## SRS 5.0 Changelog
 
+* v5.0, 2023-01-01, For [#3349](https://github.com/ossrs/srs/issues/3349): API: Fix duplicated on_stop callback event bug. v5.0.125
 * v5.0, 2022-12-31, GB28181: Enable regression test for gb28181. v5.0.122
 * v5.0, 2022-12-31, Refine configure to guess OS automatically. v5.0.121
 * v5.0, 2022-12-31, Refine default config file for SRS. v5.0.120

diff --git a/trunk/src/app/srs_app_http_static.cpp b/trunk/src/app/srs_app_http_static.cpp
@@ -70,6 +70,8 @@ SrsHlsStream::~SrsHlsStream()
 
 srs_error_t SrsHlsStream::serve_m3u8_ctx(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, ISrsFileReaderFactory* factory, string fullpath, SrsRequest* req, bool* served)
 {
+    srs_error_t err = srs_success;
+
     string ctx = r->query_get(SRS_CONTEXT_IN_HLS);
 
     // If HLS stream is disabled, use SrsHttpFileServer to serve HLS, which is normal file server.
@@ -82,9 +84,6 @@ srs_error_t SrsHlsStream::serve_m3u8_ctx(ISrsHttpResponseWriter* w, ISrsHttpMess
     // @remark Be careful that the stream has extension now, might cause identify fail.
     req->stream = srs_path_basename(r->path());
 
-    // Always make the ctx alive now.
-    alive(ctx, req);
-
     // Served by us.
     *served = true;
 
@@ -95,11 +94,16 @@ srs_error_t SrsHlsStream::serve_m3u8_ctx(ISrsHttpResponseWriter* w, ISrsHttpMess
             *served = false;
             return srs_success;
         }
-        return serve_exists_session(w, r, factory, fullpath);
+        err = serve_exists_session(w, r, factory, fullpath);
+    } else {
+        // Create a m3u8 in memory, contains the session id(ctx).
+        err = serve_new_session(w, r, req, ctx);
     }
 
-    // Create a m3u8 in memory, contains the session id(ctx).
-    return serve_new_session(w, r, req);
+    // Always make the ctx alive now.
+    alive(ctx, req);
+
+    return err;
 }
 
 void SrsHlsStream::on_serve_ts_ctx(ISrsHttpResponseWriter* w, ISrsHttpMessage* r)
@@ -124,14 +128,13 @@ void SrsHlsStream::on_serve_ts_ctx(ISrsHttpResponseWriter* w, ISrsHttpMessage* r
     SrsStatistic::instance()->kbps_add_delta(ctx, delta);
 }
 
-srs_error_t SrsHlsStream::serve_new_session(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, SrsRequest* req)
+srs_error_t SrsHlsStream::serve_new_session(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, SrsRequest* req, std::string& ctx)
 {
     srs_error_t err = srs_success;
 
     SrsHttpMessage* hr = dynamic_cast<SrsHttpMessage*>(r);
     srs_assert(hr);
 
-    string ctx;
     if (ctx.empty()) {
         // make sure unique
         do {

diff --git a/trunk/src/app/srs_app_http_static.hpp b/trunk/src/app/srs_app_http_static.hpp
@@ -34,7 +34,7 @@ class SrsHlsStream : public ISrsFastTimer
     virtual srs_error_t serve_m3u8_ctx(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, ISrsFileReaderFactory* factory, std::string fullpath, SrsRequest* req, bool* served);
     virtual void on_serve_ts_ctx(ISrsHttpResponseWriter* w, ISrsHttpMessage* r);
 private:
-    srs_error_t serve_new_session(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, SrsRequest *req);
+    srs_error_t serve_new_session(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, SrsRequest *req, std::string& ctx);
     srs_error_t serve_exists_session(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, ISrsFileReaderFactory* factory, std::string fullpath);
     bool ctx_is_exist(std::string ctx);
     void alive(std::string ctx, SrsRequest* req);

diff --git a/trunk/src/core/srs_core_version5.hpp b/trunk/src/core/srs_core_version5.hpp
@@ -9,6 +9,6 @@
 
 #define VERSION_MAJOR       5
 #define VERSION_MINOR       0
-#define VERSION_REVISION    124
+#define VERSION_REVISION    125
 
 #endif
diff --git a/trunk/src/utest/srs_utest_http.cpp b/trunk/src/utest/srs_utest_http.cpp
@@ -144,6 +144,18 @@ string mock_http_response3(int status, string content)
     return ss.str();
 }
 
+string mock_http_response4(int status, string content)
+{
+    string m3u8_header = "#EXTM3U\n#EXT-X-STREAM-INF:BANDWIDTH=1,AVERAGE-BANDWIDTH=1\n";
+
+    stringstream ss;
+    ss << "HTTP/1.1 " << status << " " << srs_generate_http_status_text(status) << "\r\n"
+        << "Content-Length: " << content.length() + m3u8_header.length() << "\r\n\r\n"
+        << m3u8_header
+        << content;
+    return ss.str();
+}
+
 bool is_string_contain(string substr, string str)
 {
     return (string::npos != str.find(substr));
@@ -1286,7 +1298,11 @@ VOID TEST(ProtocolHTTPTest, VodStreamHandlers)
         HELPER_ASSERT_SUCCESS(r.set_url("/index.m3u8?hls_ctx=123456", false));
 
         HELPER_ASSERT_SUCCESS(h.serve_http(&w, &r));
-        __MOCK_HTTP_EXPECT_STREQ(200, "Hello, world!", w);
+        __MOCK_HTTP_EXPECT_STREQ4(200, "/index.m3u8?hls_ctx=123456", w);
+
+        MockResponseWriter w2;
+        HELPER_ASSERT_SUCCESS(h.serve_http(&w2, &r));
+        __MOCK_HTTP_EXPECT_STREQ(200, "Hello, world!", w2);
     }
 
     // Should return "hls_ctx"
@@ -1304,7 +1320,11 @@ VOID TEST(ProtocolHTTPTest, VodStreamHandlers)
         HELPER_ASSERT_SUCCESS(r.set_url("/index.m3u8?hls_ctx=123456", false));
 
         HELPER_ASSERT_SUCCESS(h.serve_http(&w, &r));
-        __MOCK_HTTP_EXPECT_STREQ(200, "livestream-13.ts?hls_ctx=123456", w);
+        __MOCK_HTTP_EXPECT_STREQ4(200, "/index.m3u8?hls_ctx=123456", w);
+
+        MockResponseWriter w2;
+        HELPER_ASSERT_SUCCESS(h.serve_http(&w2, &r));
+        __MOCK_HTTP_EXPECT_STREQ(200, "livestream-13.ts?hls_ctx=123456", w2);
     }
 }
 

diff --git a/trunk/src/utest/srs_utest_http.hpp b/trunk/src/utest/srs_utest_http.hpp
@@ -51,6 +51,7 @@ class MockMSegmentsReader : public ISrsReader
 
 string mock_http_response(int status, string content);
 string mock_http_response2(int status, string content);
+string mock_http_response4(int status, string content);
 bool is_string_contain(string substr, string str);
 
 #define __MOCK_HTTP_EXPECT_STREQ(status, text, w) \
@@ -59,6 +60,9 @@ bool is_string_contain(string substr, string str);
 #define __MOCK_HTTP_EXPECT_STREQ2(status, text, w) \
         EXPECT_STREQ(mock_http_response2(status, text).c_str(), HELPER_BUFFER2STR(&w.io.out_buffer).c_str())
 
+#define __MOCK_HTTP_EXPECT_STREQ4(status, text, w) \
+        EXPECT_STREQ(mock_http_response4(status, text).c_str(), HELPER_BUFFER2STR(&w.io.out_buffer).c_str())
+
 #define __MOCK_HTTP_EXPECT_STRHAS(status, text, w) \
         EXPECT_PRED2(is_string_contain, text, HELPER_BUFFER2STR(&w.io.out_buffer).c_str())