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

[fix] free memory pool in rtmp core/live server conf #150

Merged
merged 1 commit into from
Mar 4, 2020

Conversation

spacewander
Copy link
Contributor

When this module is built with ASAN, running it wil receive several
memory leak reports:

Direct leak of 48 byte(s) in 1 object(s) allocated from:
    ...
    #3 0xd6e96a in ngx_rtmp_live_merge_app_conf /home/vagrant/git/nginx-http-flv-module/ngx_rtmp_live_module.c:275:18
    #4 0xd06a87 in ngx_rtmp_block /home/vagrant/git/nginx-http-flv-module/ngx_rtmp.c:283:22
    ...

Direct leak of 48 byte(s) in 1 object(s) allocated from:
    ...
    #3 0xd3edc7 in ngx_rtmp_core_merge_srv_conf /home/vagrant/git/nginx-http-flv-module/ngx_rtmp_core_module.c:407:22
    ...

This fix is expected to mute the report.

When this module is built with ASAN, running it wil receive several
memory leak reports:

    Direct leak of 48 byte(s) in 1 object(s) allocated from:
        ...
        winshining#3 0xd6e96a in ngx_rtmp_live_merge_app_conf /home/vagrant/git/nginx-http-flv-module/ngx_rtmp_live_module.c:275:18
        winshining#4 0xd06a87 in ngx_rtmp_block /home/vagrant/git/nginx-http-flv-module/ngx_rtmp.c:283:22
        ...

    Direct leak of 48 byte(s) in 1 object(s) allocated from:
        ...
        winshining#3 0xd3edc7 in ngx_rtmp_core_merge_srv_conf /home/vagrant/git/nginx-http-flv-module/ngx_rtmp_core_module.c:407:22
        ...

This fix is expected to mute the report.
ngx_rtmp_live_app_conf_t *lacf;

lacf = ngx_pcalloc(cf->pool, sizeof(ngx_rtmp_live_app_conf_t));
if (lacf == NULL) {
return NULL;
}

cln = ngx_pool_cleanup_add(cf->pool, 0);
Copy link
Owner

@winshining winshining Mar 3, 2020

Choose a reason for hiding this comment

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

Placing these code snippets in ngx_rtmp_live_merge_app_conf will fix the memory leak?
Doing so keeps style consistent to the above one.

@winshining
Copy link
Owner

Would you please test what I commented?
Thank you very much!

@winshining winshining merged commit a2a1484 into winshining:master Mar 4, 2020
@spacewander
Copy link
Contributor Author

@winshining
Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants