Edge: Improve stability for state and fd closing. v5.0.214 v6.0.139 (#…

…4126) 1. Should always stop coroutine before close fd, see #511, #1784 2. When edge forwarder coroutine quit, always set the error code. 3. Do not unpublish if invalid state. --------- Co-authored-by: Jacob Su <[email protected]>
ossrs · Jul 24, 2024 · f04e939 · f04e939
1 parent 8c034e5
commit f04e939
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 16 deletions.
diff --git a/trunk/doc/CHANGELOG.md b/trunk/doc/CHANGELOG.md
@@ -7,6 +7,7 @@ The changelog for SRS.
 <a name="v6-changes"></a>
 
 ## SRS 6.0 Changelog
+* v6.0, 2024-07-24, Merge [#4126](https://github.com/ossrs/srs/pull/4126): Edge: Improve stability for state and fd closing. v6.0.139 (#4126)
 * v6.0, 2024-07-13, Merge [#4111](https://github.com/ossrs/srs/pull/4111): DASH: Fix time unit error for disposing. v6.0.138 (#4111)
 * v6.0, 2024-07-09, Merge [#4028](https://github.com/ossrs/srs/pull/4028): HTTPS: Support config key/cert for HTTPS API. v6.0.137 (#4028)
 * v6.0, 2024-07-09, Merge [#4109](https://github.com/ossrs/srs/pull/4109): UniquePtr: Support SrsUniquePtr to replace SrsAutoFree. v6.0.136 (#4109)
@@ -149,6 +150,7 @@ The changelog for SRS.
 <a name="v5-changes"></a>
 
 ## SRS 5.0 Changelog
+* v5.0, 2024-07-24, Merge [#4126](https://github.com/ossrs/srs/pull/4126): Edge: Improve stability for state and fd closing. v5.0.214 (#4126)
 * v5.0, 2024-06-03, Merge [#4057](https://github.com/ossrs/srs/pull/4057): RTC: Support dropping h.264 SEI from NALUs. v5.0.213 (#4057)
 * v5.0, 2024-04-23, Merge [#4038](https://github.com/ossrs/srs/pull/4038): RTMP: Do not response publish start message if hooks fail. v5.0.212 (#4038)
 * v5.0, 2024-04-22, Merge [#4033](https://github.com/ossrs/srs/pull/4033): issue #3967: support x509 certification chiain in single pem file. v5.0.211 (#4033)

diff --git a/trunk/src/app/srs_app_edge.cpp b/trunk/src/app/srs_app_edge.cpp
@@ -782,6 +782,10 @@ srs_error_t SrsEdgeForwarder::start()
 
         url = srs_generate_rtmp_url(server, port, req->host, vhost, req->app, req->stream, req->param);
     }
+
+    // We must stop the coroutine before disposing the sdk.
+    srs_freep(trd);
+    trd = new SrsSTCoroutine("edge-fwr", this, _srs_context->get_id());
 
     // open socket.
     srs_freep(sdk);
@@ -806,10 +810,8 @@ srs_error_t SrsEdgeForwarder::start()
     if ((err = sdk->publish(_srs_config->get_chunk_size(req->vhost), false, &stream)) != srs_success) {
         return srs_error_wrap(err, "sdk publish");
     }
-
-    srs_freep(trd);
-    trd = new SrsSTCoroutine("edge-fwr", this, _srs_context->get_id());
-
+
+    // Start the forwarding coroutine.
     if ((err = trd->start()) != srs_success) {
         return srs_error_wrap(err, "coroutine");
     }
@@ -821,9 +823,12 @@ srs_error_t SrsEdgeForwarder::start()
 
 void SrsEdgeForwarder::stop()
 {
+    // Make sure the coroutine is stopped before disposing the sdk,
+    // for sdk is used by coroutine.
     trd->stop();
-    queue->clear();
     srs_freep(sdk);
+
+    queue->clear();
 }
 
 // when error, edge ingester sleep for a while and retry.
@@ -841,6 +846,11 @@ srs_error_t SrsEdgeForwarder::cycle()
         }
 
         if ((err = do_cycle()) != srs_success) {
+            // If cycle stopping, we should always set the quit error code.
+            if (send_error_code == 0) {
+                send_error_code = srs_error_code(err);
+            }
+
             return srs_error_wrap(err, "do cycle");
         }
 

diff --git a/trunk/src/app/srs_app_rtmp_conn.cpp b/trunk/src/app/srs_app_rtmp_conn.cpp
@@ -946,25 +946,22 @@ srs_error_t SrsRtmpConn::publishing(SrsSharedPtr<SrsLiveSource> source)
     }
 
     // TODO: FIXME: Should refine the state of publishing.
-    if ((err = acquire_publish(source)) == srs_success) {
+    srs_error_t acquire_err = acquire_publish(source);
+    if ((err = acquire_err) == srs_success) {
         // use isolate thread to recv,
         // @see: https://github.com/ossrs/srs/issues/237
         SrsPublishRecvThread rtrd(rtmp, req, srs_netfd_fileno(stfd), 0, this, source, _srs_context->get_id());
         err = do_publishing(source, &rtrd);
         rtrd.stop();
     }
 
-    // whatever the acquire publish, always release publish.
-    // when the acquire error in the midlle-way, the publish state changed,
-    // but failed, so we must cleanup it.
-    // @see https://github.com/ossrs/srs/issues/474
-    // @remark when stream is busy, should never release it.
-    if (srs_error_code(err) != ERROR_SYSTEM_STREAM_BUSY) {
+    // Release and callback when acquire publishing success, if not, we should ignore, because the source
+    // is not published by this session.
+    if (acquire_err == srs_success) {
         release_publish(source);
+        http_hooks_on_unpublish();
     }
 
-    http_hooks_on_unpublish();
-
     return err;
 }
 

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    213
+#define VERSION_REVISION    214
 
 #endif
diff --git a/trunk/src/core/srs_core_version6.hpp b/trunk/src/core/srs_core_version6.hpp
@@ -9,6 +9,6 @@
 
 #define VERSION_MAJOR       6
 #define VERSION_MINOR       0
-#define VERSION_REVISION    138
+#define VERSION_REVISION    139
 
 #endif