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 reusable thread need join #559

Merged
merged 2 commits into from
Jan 9, 2016
Merged

srs reusable thread need join #559

merged 2 commits into from
Jan 9, 2016

Conversation

lemonbinary
Copy link

@lemonbinary lemonbinary commented Jan 9, 2016

Let me describe the problem in detail.
In the function SrsThread::stop(), the variable "loop" is set to false first, and then the function SrsThread::dispose() is executed.
In the function SrsThread::thread_cycle(), the variable "disposed" is set to true when exiting the loop.
In this way, it is possible for the variable "disposed" to be assigned true before executing SrsThread::dispose(), causing the function SrsThread::dispose() to directly return.

There is another issue. I commented out a line of code. The description above is for issue #546. I think the occurrence of issue #546 is because the assignment of "disposed" is executed after handler->on_thread_stop(). For SrsConnection, deleting itself is done in handler->on_thread_stop(), which means freeing the current thread. Therefore, any operation after handler->on_thread_stop() will result in an error.


TRANS_BY_GPT3

@winlinvip
Copy link
Member

winlinvip commented Jan 9, 2016

This is because if the thread exits on its own, there is no need to go through the complicated process of disposing, waiting for the thread to exit, and confirming the exit, etc.
The issue you mentioned is indeed a bug. It should be changed as follows:

  1. If it is an active exit, that is, calling stop to exit, it should be confirmed that the thread has exited as it is now.
  2. If the thread exits on its own, as it is currently, then disposed=true should be set.
    Both of these methods should call the third function: st_release, to release resources using join.

TRANS_BY_GPT3

@winlinvip
Copy link
Member

winlinvip commented Jan 9, 2016

Or in the second case, you can actively call join:

        // when joinable, wait util quit.
        if (_joinable) {
            // wait the thread to exit.
            int ret = st_thread_join(tid, NULL);
            if (ret) {
                srs_warn("core: ignore join thread failed.");
            }
        }

Then set disposed=true.

TRANS_BY_GPT3

@lemonbinary
Copy link
Author

lemonbinary commented Jan 9, 2016

Can join be called in the current thread? It should be called in another thread, right?

TRANS_BY_GPT3

@winlinvip
Copy link
Member

winlinvip commented Jan 9, 2016

Makes sense, I think you just removed these few lines. It will run again in the dispose, so it's not a problem.

TRANS_BY_GPT3

@lemonbinary
Copy link
Author

lemonbinary commented Jan 9, 2016

Yeah, I feel the same way. If there's no need to join, just skip it. And the while loop below will also exit directly, so there's no significant consumption.

TRANS_BY_GPT3

@winlinvip
Copy link
Member

👍

Please update the pr.
Good job~

@winlinvip
Copy link
Member

This pr fixed the #553

winlinvip added a commit that referenced this pull request Jan 9, 2016
srs reusable thread need join
@winlinvip winlinvip merged commit da722b5 into ossrs:2.0release Jan 9, 2016
winlinvip added a commit that referenced this pull request Jan 9, 2016
@winlinvip
Copy link
Member

Merged to develop: eac5440

winlinvip added a commit that referenced this pull request Jan 11, 2016
@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.

2 participants