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

When multiple push/pull stream links of the same stream arrive at the same time, under the condition of enabling the DVR or HLS module, multiple SrsSource may be created for the same stream, leading to memory leaks! #1230

Closed
dean-river opened this issue Sep 24, 2018 · 2 comments
Assignees
Labels
Bug It might be a bug. TransByAI Translated by AI/GPT.
Milestone

Comments

@dean-river
Copy link

dean-river commented Sep 24, 2018

The code is as follows:
int SrsSource::fetch_or_create(SrsRequest* r, ISrsSourceHandler* h, SrsSource** pps)
{
int ret = ERROR_SUCCESS;

SrsSource* source = NULL;
if ((source = fetch(r)) != NULL) {
    *pps = source;
    return ret;
}

string stream_url = r->get_stream_url();
string vhost = r->vhost;

// should always not exists for create a source.
srs_assert (pool.find(stream_url) == pool.end());

**source = new SrsSource();
if ((ret = source->initialize(r, h)) != ERROR_SUCCESS) {
    srs_freep(source);
    return ret;
}
    
pool[stream_url] = source;**
srs_info("create new source for url=%s, vhost=%s", stream_url.c_str(), vhost.c_str());

*pps = source;

return ret;

}
The following description corresponds to the premise of enabling the DVR and HLS modules.

Above, when the SrsSource object is not found, a new one is created, but it is not immediately put into the pool. Instead, it needs to be initialized. In the initialize function, the DVR and HLS modules are initialized. In the initialize functions of these two modules, the start function of SrsAsyncCallWorker is called, which starts a new coroutine. This will cause a switch to the cycle function of SrsAsyncCallWorker, but it is not necessarily switched back to the coroutine that initialized SrsSource. This means that if a new connection arrives and enters this function, it may create SrsSource again.

In conclusion, the source->initialize function cannot guarantee coroutine-level atomicity, which can lead to this situation.

I have thought of three solutions:

  1. Restrict the initialize function from switching coroutines. Move the start function of SrsAsyncCallWorker to another place, such as the execute function. However, this cannot guarantee that the execute operation does not switch coroutines. Moreover, initialize may perform a lot of work, so this restriction seems not ideal.
  2. Add a mutex lock to the code segment that creates SrsSource and puts it into the pool. However, this will reduce the performance of simultaneously accessing connections.
  3. After initialization is completed and before putting it into the pool, check again if it already exists in the pool. If it does, it means that another coroutine has completed initialization first, so delete the one created by itself and use the one that was put into the pool first. Currently, I am using this method. However, the code looks a bit awkward.

I hope the experts have more elegant solutions to solve this problem!!!

TRANS_BY_GPT3

@winlinvip winlinvip added the Bug It might be a bug. label Oct 7, 2018
@winlinvip winlinvip added this to the srs 3.0 release milestone Oct 7, 2018
@winlinvip
Copy link
Member

winlinvip commented Oct 7, 2018

The competition condition of Source is a problem. This problem was previously overlooked, but essentially it is a problem of thread or coroutine synchronization.

TRANS_BY_GPT3

@winlinvip
Copy link
Member

winlinvip commented Jan 28, 2020

If there are asynchronous operations during source.initialize, causing ST coroutine switching, it is possible to have multiple Sources for one URL, for example:

(gdb) p stream_url
$2 = "/live/livestream"
(gdb) p source
$3 = (SrsSource *) 0xa66de0

(gdb) p stream_url
$4 = "/live/livestream"
(gdb) p source
$5 = (SrsSource *) 0xa713b0

SrsSourceManager::fetch_or_create needs to be protected by a mutex to prevent ST coroutine switching.

TRANS_BY_GPT3

@winlinvip winlinvip self-assigned this Sep 11, 2021
@winlinvip winlinvip changed the title 当对同一路流的推/拉流链接同时到来时,在开启 dvr 或者 hls 模块的前提下,同一个SrsSource 可能创建多个,导致内存泄漏! When multiple push/pull stream links of the same stream arrive at the same time, under the condition of enabling the DVR or HLS module, multiple SrsSource may be created for the same stream, leading to memory leaks! Jul 28, 2023
@winlinvip winlinvip added the TransByAI Translated by AI/GPT. label Jul 28, 2023
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

2 participants