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: Fix the issue of srs randomly failing to start during machine startup. #2012

Closed
wants to merge 3 commits into from

Conversation

PieerePi
Copy link
Contributor

@PieerePi PieerePi commented Nov 2, 2020

srs: Fix the issue of srs randomly failing to start during machine startup.


TRANS_BY_GPT3

感觉好像是son进程还在SrsDtlsCertificate::initialize过程中就直接退出了;
让father进程等一下再退出就好了;
手工启动srs好像没有此问题。
@codecov-io
Copy link

codecov-io commented Nov 2, 2020

Codecov Report

Merging #2012 (6c535ea) into develop (a28f985) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #2012   +/-   ##
========================================
  Coverage    57.17%   57.17%           
========================================
  Files          125      125           
  Lines        52707    52707           
========================================
+ Hits         30134    30135    +1     
+ Misses       22573    22572    -1     
Impacted Files Coverage Δ
trunk/src/protocol/srs_service_utility.cpp 72.97% <0.00%> (+0.54%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a28f985...6c535ea. Read the comment docs.

TRANS_BY_GPT3

@winlinvip winlinvip changed the base branch from 4.0release to develop November 15, 2020 15:27
@@ -450,6 +450,7 @@ srs_error_t run_directly_or_daemon()
}

if(pid > 0) {
sleep(3);
Copy link
Member

@winlinvip winlinvip Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have to wait for 3 seconds? Can you provide some information?

TRANS_BY_GPT3

Copy link
Contributor Author

@PieerePi PieerePi Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OpenSSL has some issues with shared processes between parent and child. Some references are provided for your reference.
https://wiki.openssl.org/index.php/Random_fork-safety
https://www.openssl.org/docs/man1.1.1/man3/OPENSSL_fork_child.html (These new methods are available after 1.1.1, but currently srs is using 1.1.0e.)
In this case, I am asking the parent process to wait for a while, allowing the child process to successfully complete _srs_rtc_dtls_certificate->initialize before the parent process exits. Otherwise, there is a high probability of _srs_rtc_dtls_certificate->initialize failing, and srs cannot start with the machine.

Waiting for 3 seconds in the parent process is just a workaround and there may be better methods available. For example, the approach used by Janus is to have the parent process wait until the child process has finished initializing before exiting.
janus.c:4124, /* Ok, we're the parent: let's wait for the child to tell us everything started fine */
Is this problem only present when there is WebRTC functionality?

TRANS_BY_GPT3

Copy link
Member

@winlinvip winlinvip Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution also solves the problem of initializing openssl, using sleep is definitely a temporary solution. Let me see how to solve it completely.

TRANS_BY_GPT3

Copy link

@shitizenlism shitizenlism Jan 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, not working!

TRANS_BY_GPT3

Copy link
Contributor Author

@PieerePi PieerePi Jan 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, not working!

Oh, it's possible that we're not dealing with the same issue. Please try another method.

TRANS_BY_GPT3

@winlinvip
Copy link
Member

winlinvip commented Oct 26, 2021

https://www.openssl.org/docs/man1.1.1/man3/OPENSSL_fork_child.html

Platforms without fork(2) will probably not need to use these functions. Platforms with fork(2) but without pthread_atfork(3) will probably need to call them manually, as described in the following paragraph. Platforms such as Linux that have both functions will normally not need to call these functions as the OpenSSL library will do so automatically.

It seems that Linux has no problem.

TRANS_BY_GPT3

@winlinvip winlinvip closed this Oct 26, 2021
@winlinvip
Copy link
Member

winlinvip commented Oct 26, 2021

Although this PR is not being merged, thank you very much @PieerePi.

TRANS_BY_GPT3

@winlinvip winlinvip changed the title srs:修复srs随机器启动但有时启动不成功的问题 srs: Fix the issue of srs randomly failing to start during machine startup. Jul 29, 2023
@winlinvip winlinvip added the TransByAI Translated by AI/GPT. label Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TransByAI Translated by AI/GPT.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants