[core] Fixed bug: tsbpd might miss m_bClosing flag setting, flag not always properly set. #1709
Conversation
…always properly set. Added some sanity checks
srtcore/core.cpp
Outdated
| @@ -5550,7 +5561,7 @@ void * srt::CUDT::tsbpd(void* param) | |||
| if (self->m_bClosing) | |||
| break; | |||
|
|
|||
| SRT_ATR_UNUSED bool bWokeUpOnSignal = true; | |||
| bool bWokeUpOnSignal = true; | |||
There was a problem hiding this comment.
The scope of this variable can be reduced. It is used both in the if and else statements, but independently. Thus can be defined inside those.
There was a problem hiding this comment.
You are right, this was likely tied to some earlier version that provided some log instruction after this if-branch, hence the scope was bigger. Now it has its own log in every branch, so this isn't necessary.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1709 +/- ##
==========================================
+ Coverage 65.75% 68.63% +2.87%
==========================================
Files 101 101
Lines 17966 18327 +361
==========================================
+ Hits 11814 12579 +765
+ Misses 6152 5748 -404 ☔ View full report in Codecov by Sentry. |
|
Tested on a Raspberry Pi3 alike platform. This PR SRT v1.5.4 master 2024-12-12 f109fb1 |
|
Second run. This PR. Master 2024-12-12 f109fb1 |
|
Third run This PR. Master 2024-12-12 f109fb1 |
|
Would you be able to check also a similar version, just in place where there's a wait on Hard to believe that this is the point of performance degradation, but there are about 3 candidates among the changes that could cause it. I also hope that you made the measurements on C++11 version. |
Fixed:
releaseSyncNote: the sanity check to make the wait on tsbpd_cc interrupted once per a time (125ms) that had to prevent the deadlock was removed, as it has significantly burdened the CPU usage of the TsbPd thread.