chenBright commented on code in PR #3543:
URL: https://github.com/apache/brpc/pull/3543#discussion_r4028798722


##########
test/brpc_rtmp_unittest.cpp:
##########
@@ -289,16 +296,26 @@ class PlayingDummyStream : public brpc::RtmpServerStream {
     void OnStop() {
         LOG(INFO) << "OnStop of PlayingDummyStream=" << this;
         if (_state.exchange(STATE_STOPPED) == STATE_PLAYING) {
-            bthread_stop(_play_thread);
-            bthread_join(_play_thread, nullptr);
+            // A send failure can invoke this callback on the sender bthread
+            // itself. Stopping or joining the current bthread is unsafe and
+            // interferes with its unwinding, and the sender's own reference
+            // already keeps the stream alive until SendData returns, so only
+            // stop and join when running on a different bthread.
+            if (_play_thread != bthread_self()) {
+                bthread_stop(_play_thread);
+                bthread_join(_play_thread, nullptr);
+            }

Review Comment:
   Fixed by always calling bthread_stop(_play_thread) — it only sets the stop 
flag that SendData checks, so it is safe and required on the current bthread — 
and skipping only bthread_join on the self-callback path. The sender's own 
reference keeps the stream alive until SendData returns.
   
   There is a single class with this sender-loop pattern (PlayingDummyStream); 
the other OnStop overrides do not start a sender, so this one change covers the 
reported line as well. Verified locally: all RtmpTest.* cases pass.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to