-
Notifications
You must be signed in to change notification settings - Fork 866
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core] Fixed bug: tsbpd might miss m_bClosing flag setting, flag not always properly set. #1709
base: master
Are you sure you want to change the base?
[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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. SRT v1.5.4 master 2024-12-12 f109fb1 |
Second run. Master 2024-12-12 f109fb1 |
Third run 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:
releaseSync
Added some sanity checks