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

Edge: Improve stability for state and fd closing. v5.0.214 v6.0.139 #4126

Merged
merged 3 commits into from
Jul 24, 2024

Conversation

winlinvip
Copy link
Member

@winlinvip winlinvip commented Jul 23, 2024

  1. Should always stop coroutine before close fd, see Edge pull, the system often encounters errors ret=1018 (Device or resource busy) and ret=1018 (No such file or directory). #511, When the SrsHttpApi object is destructed, the program encounters a coredump, caused by srs_close_stfd. #1784
  2. When edge forwarder coroutine quit, always set the error code.
  3. Do not unpublish if invalid state.

Co-authored-by: Jacob Su [email protected]

@winlinvip winlinvip marked this pull request as ready for review July 23, 2024 08:50
Copy link
Contributor

@suzp1984 suzp1984 left a comment

Choose a reason for hiding this comment

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

review send_error_code problem.

if ((err = do_cycle()) != srs_success) {
// If coroutine stopping, we should always set the quit error code.
err = do_cycle();
if (send_error_code == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (send_error_code == 0) {
if ((err = do_cycle()) != srs_success) {
if (send_error_code == ERROR_SUCCESS) {
// If coroutine stopping, we should always set the quit error code.
send_error_code = srs_error_code(err);
}
return srs_error_wrap(err, "do cycle");
}

suggest another way.

About send_error_code, I believe there is a dead loop:

while (true) {
if ((err = trd->pull()) != srs_success) {
return srs_error_wrap(err, "edge forward pull");
}
if (send_error_code != ERROR_SUCCESS) {
srs_usleep(SRS_EDGE_FORWARDER_TIMEOUT);
continue;
}
// read from client.
if (true) {
SrsCommonMessage* msg = NULL;
err = sdk->recv_message(&msg);
if (err != srs_success && srs_error_code(err) != ERROR_SOCKET_TIMEOUT) {
srs_error("edge push get server control message failed. err=%s", srs_error_desc(err).c_str());
send_error_code = srs_error_code(err);
srs_error_reset(err);
continue;
}

there are no place to set send_error_code = ERROR_SUCCESS in the loop, only set the value when sdk->recv_message(&msg).

Copy link
Member Author

Choose a reason for hiding this comment

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

not dead loop, the publish owner will stop it when error.

@winlinvip winlinvip changed the title Edge: Improve stability for state and fd closing. Edge: Improve stability for state and fd closing. v5.0.214 v6.0.139 Jul 24, 2024
@winlinvip winlinvip added the RefinedByAI Refined by AI/GPT. label Jul 24, 2024
@winlinvip winlinvip merged commit f04e939 into ossrs:develop Jul 24, 2024
17 checks passed
winlinvip added a commit that referenced this pull request Jul 24, 2024
1. Should always stop coroutine before close fd, see #511, #1784
2. When edge forwarder coroutine quit, always set the error code.
3. Do not unpublish if invalid state.

---------

Co-authored-by: Jacob Su <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RefinedByAI Refined by AI/GPT.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants