-
Notifications
You must be signed in to change notification settings - Fork 851
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: srt_accept failure may make accepted socket leak #1884
[core] Fixed bug: srt_accept failure may make accepted socket leak #1884
Conversation
srtcore/api.cpp
Outdated
{ | ||
// Check if the length of the buffer to fill the name in | ||
// was large enough. | ||
const int len = b->second.size(); |
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.
Not using m_PeerAddr
to avoid locking s->m_ControlLock
?
Things have changed a bit, please merge #2681 first. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1884 +/- ##
==========================================
+ Coverage 67.20% 67.73% +0.53%
==========================================
Files 102 103 +1
Lines 20269 20686 +417
==========================================
+ Hits 13622 14012 +390
- Misses 6647 6674 +27 ☔ View full report in Codecov by Sentry. |
When the socket picked up from the accept queue turns out to have address that cannot be written back to the sockaddr buffer because it's too small, this function returns an error, but the socket remains valid and could leak this way. As this socket can no longer be returned, it must be set broken. Although theoretically this call could be repeated with fixed value, it's unlikely that an application can do a repeated call and inteligently fix the problem in runtime; instead it will rather fail completely and this way the socket will be left leaked.
Draft until: #2681 (prematurely merged)