-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
bugfix: SrsMetaCache memleak; getaddrinfo use delete. #2880
Conversation
@@ -171,29 +171,32 @@ srs_error_t srs_tcp_connect(string server, int port, srs_utime_t tm, srs_netfd_t | |||
hints.ai_socktype = SOCK_STREAM; | |||
|
|||
addrinfo* r = NULL; | |||
SrsAutoFree(addrinfo, r); |
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.
Why can't we use SrsAutoFree
?
If you need to use free
instead of delete
for deallocation, you can use SrsAutoFreeF
.
Automatic freeing can significantly improve code readability.
TRANS_BY_GPT3
When looking at the man page for getaddrinfo, it states that freeaddrinfo should be used to release the memory.
The memory allocated by getaddrinfo may not necessarily be allocated using new, so it is inappropriate to use delete with SrsAutoFree.
Furthermore, the addrinfo allocated within getaddrinfo is a linked list, so SrsAutoFree cannot release the entire addrinfo chain at once.
At 2022-01-13 12:20:28, "Winlin" ***@***.***> wrote:
@winlinvip commented on this pull request.
In trunk/src/protocol/srs_service_st.cpp:
@@ -171,29 +171,32 @@ srs_error_t srs_tcp_connect(string server, int port, srs_utime_t tm, srs_netfd_t
hints.ai_socktype = SOCK_STREAM;
addrinfo* r = NULL;
- SrsAutoFree(addrinfo, r);
Why can't we use SrsAutoFree?
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you authored the thread.Message ID: ***@***.***>
`TRANS_BY_GPT3`
|
If it is not possible to release using free or delete, you must use freeaddrinfo to release. In that case, it can be changed like this:
Transform the resource allocation and handling into a sub-function, and then release it once at the calling location. If you find it troublesome, you can use the goto method in C, you can refer to FFmpeg for specific details.
|
There is another way, to improve
|
I add a macro
Please use this macro to free the addrinfo, does it OK? |
@bluestn Could you please make the changes according to the new modifications? Can you also merge the 4.0 release branch?
|
@bluestn If you don't know how to operate git, you can contact me on WeChat. There is no need to resubmit a new PR, you can continuously update this PR.
|
Codecov Report
@@ Coverage Diff @@
## 4.0release #2880 +/- ##
==============================================
- Coverage 60.22% 60.21% -0.01%
==============================================
Files 121 121
Lines 51076 51078 +2
==============================================
Hits 30758 30758
- Misses 20318 20320 +2 | Impacted Files | Coverage Δ | |' Translated to English while maintaining the markdown structure: '| Impacted Files | Coverage Δ | | ' Translated to English while maintaining the markdown structure: '| trunk/src/kernel/srs_kernel_utility.cpp | Continue to review full report at Codecov.
Translated to English while maintaining the markdown structure: '|
|
Summary