Skip to content
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

srs occasional crash caused by cleaning up unused SrsSource mechanism #713

Closed
haofz opened this issue Dec 13, 2016 · 7 comments
Closed

srs occasional crash caused by cleaning up unused SrsSource mechanism #713

haofz opened this issue Dec 13, 2016 · 7 comments
Assignees
Labels
Bug It might be a bug. TransByAI Translated by AI/GPT.
Milestone

Comments

@haofz
Copy link
Contributor

haofz commented Dec 13, 2016

After adding the mechanism to clean up useless SrsSource,
https://github.com/ossrs/srs/commit/c7b97aa1c3de380451e2d229fc5bef702e2d264d
https://github.com/ossrs/srs/commit/590e9517398c4d442bcf125421353b4b338217d1
During testing, a crash issue was found in the Srs program. The log is as follows:
2016-12-13 14:16:42.790|trace|6104|243876|0|RTMP client ip=192.168.10.20
2016-12-13 14:16:42.792|trace|6104|243876|0|complex handshake success
2016-12-13 14:16:42.792|trace|6104|243876|0|connect app, tcUrl=rtmp://172.20.1.2:1935/live, pageUrl=, swfUrl=, schema=rtmp, vhost=defaultVhost, port=1935, app=live, param=, args=null
2016-12-13 14:16:42.792|trace|6104|243876|0|out chunk size to 60000
2016-12-13 14:16:42.872|trace|6104|243876|0|input chunk size to 60000
2016-12-13 14:16:42.873|trace|6104|243876|0|client identified, type=fmle-publish, stream_name=livestream, duration=-1.00
2016-12-13 14:16:42.873|trace|6104|243876|0|update req of soruce for auth ok
2016-12-13 14:16:42.873|trace|6104|243876|0|source url=defaultVhost/live/livestream, ip=192.168.10.20, cache=1, is_edge=0, source_id=-1[-1]
2016-12-13 14:16:42.888|trace|6104|243218|0|cleanup die source, total=43
2016-12-13 14:16:42.953|trace|6104|243876|0|>>>>>>>>>>>>> FMLE start to publish stream livestream, url=defaultVhost/live/livestream.

Analysis of the cause:
There was a blockage between the connect and actual streaming during a push stream, causing the source to be cleared and resulting in a program crash.

Process:

  1. The first push stream was successful and then disconnected.
  2. After 29 seconds, the second push stream occurred, and the SrsSource was fetched using the fetch() method. In the fetch() method, die_at was not set to -1 (this is not required in the create method).
  3. There was a blockage between the connect and publishing steps, such as expect_message(). If at this moment it happened to be exactly 30 seconds, the source would be cleared by the 1-second interval cleaning mechanism.
  4. When using the same SrsSource again, it crashed.

Solution:
In SrsSource::fetch(), add source->die_at = -1; This has been personally tested and found to be effective in resolving the issue.

TRANS_BY_GPT3

@winlinvip winlinvip added this to the srs 2.0 release milestone Dec 13, 2016
@winlinvip
Copy link
Member

winlinvip commented Dec 13, 2016

The fetch mechanism of Source is not atomic, this part needs to be changed.

TRANS_BY_GPT3

@winlinvip
Copy link
Member

winlinvip commented Dec 13, 2016

The problem with it is that after fetching, it may not necessarily create a consumer or publisher. After fetching, if it is not cleaned up, it will cause the source to not be cleaned up (if there are no subsequent publishers or consumers). If the die at status is not changed after fetching, it will result in the source being cleaned up and subsequent use of the source will crash. Therefore, setting die_at=-1 in the fetch is also incorrect.

In other words, the cleanup of the source needs to consider the scenario where it is not used (neither published nor consumed) after fetch_or_create.

TRANS_BY_GPT3

@winlinvip winlinvip reopened this Dec 13, 2016
@winlinvip
Copy link
Member

winlinvip commented Dec 13, 2016

A simpler way is to remove the function of source cleaning.
Support for this feature will be added in the future.

Dup to #1509
Solution: #1579 (comment)

TRANS_BY_GPT3

@darkterrorooo
Copy link

darkterrorooo commented Dec 29, 2017

Recently, I have also been researching this memory release issue. In the latest version of srs-2.0.243, I enabled the source cleaning mechanism and conducted the following tests:

  1. I pushed 40 streams concurrently, and the memory reached 60MB. After enabling the source cleaning mechanism and ending the stream, I found that the source was indeed cleaned up. However, the memory still remained at around 60MB.

  2. I pushed 100 streams, and the memory reached around 160MB. After ending the stream, I found that the source was also cleaned up. However, the memory will still remain at around 60MB.

So I have a question: Does SRS have a memory retention mechanism? To retain a certain amount of memory to prevent repetitive allocation? I have been searching for a while but couldn't find where it is. I would greatly appreciate it if someone could enlighten me.

Finally, thanks to Winlinvip for their open-source contribution!

TRANS_BY_GPT3

@winlinvip
Copy link
Member

winlinvip commented Jan 4, 2018

2.0 has already disabled the cleaning of Source, which will be implemented in 3.0. Is it the modification of the code that caused the crash when enabling source cleaning? Currently, this feature requires more modifications to be supported, so it was disabled in 2.0.

TRANS_BY_GPT3

@winlinvip
Copy link
Member

winlinvip commented Mar 4, 2021

Dup to #1509
Solution: #1579 (comment)

@winlinvip winlinvip self-assigned this Sep 18, 2021
@winlinvip winlinvip added the Bug It might be a bug. label Sep 18, 2021
@winlinvip winlinvip changed the title srs偶发崩溃,由清理无用SrsSource机制造成的 srs occasional crash caused by cleaning up unused SrsSource mechanism Jul 28, 2023
@winlinvip winlinvip added the TransByAI Translated by AI/GPT. label Jul 28, 2023
@winlinvip
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug It might be a bug. TransByAI Translated by AI/GPT.
Projects
None yet
Development

No branches or pull requests

3 participants