[core] Removed unnecessary post-cleaning of closed socket in the cleanup.#3327
Open
ethouris wants to merge 7 commits into
Open
[core] Removed unnecessary post-cleaning of closed socket in the cleanup.#3327ethouris wants to merge 7 commits into
ethouris wants to merge 7 commits into
Conversation
added 2 commits
June 2, 2026 12:25
…moved cleaning up sockets after GC exit. Added description
Collaborator
stevomatthews
left a comment
There was a problem hiding this comment.
Looks good as is.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem:
After the proper cleanup has been added to handle the fork case, the cleanup function, which also executes from the global destructor, does a usual socket cleanup, which was so far done only inside the GC thread, and it doesn't remove a socket that is still in the
CRcvUListbecause that's a marker that theCRcvQueue::workerthread is still using it. In case when in a single snapshot of the library state all threads suddenly disappear - which may happen under certain circumstances - the cleanup function catches a situation of a stuck socket, for which there's no one left to free it.We state that as long as the GC thread is running, it takes care of all sockets. When it is asked to quit by the
stopGarbageCollector()call, it should completely close and delete all sockets. ThecloseAllSockets()then is run only "just in case", but in this situation it won't work properly if there are any sockets left that somehow the GC thread didn't delete.Solution:
The post-cleanup of the sockets has been removed. For SRT library it is believed that if the GC thread has been joined, all sockets have been deleted and there's no need to do any post-checks here. If anything is still not cleaned up at that point, it simply means that there is some data corruption caused by the misuse of the library rules, so any kind of action would end up with even worse kind of undefined behavior - hence it's best and safest to do nothing in this situation.
There's also more explanations added in the documentation to the necessary call to
srt_cleanup()and the cleanup facility done in the global destructor.