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

Handle leak; memory leak #11

Open
ntfshard opened this issue Nov 15, 2016 · 4 comments
Open

Handle leak; memory leak #11

ntfshard opened this issue Nov 15, 2016 · 4 comments

Comments

@ntfshard
Copy link
Contributor

Hello

I guess in master/src/node_mpg123.cc in line: 51 your mpg123_handle *mh leaks;
IMHO you should destroy it in dtor with mpg123_delete(mpg123_handle *);

BTW it looks like your loop_data pointer from line: 60 doesn't have a corresponding delete
IMHO dynamic allocation is not needed here =)

P.S. you can delete/free a nullptr -- it's ok (according to C++ standard):
n3797:5.3.5_2
... In the first alternative (delete object), the value of the operand of delete may be a null pointer value, a pointer to a non-array object created by a previous new-expression, or a pointer to a subobject...

So you can avoid using condition like in line 122

--M

@bendi
Copy link
Owner

bendi commented Nov 15, 2016

Great thanks for pointing it out! Would you have time to prepare pull request?

@ntfshard
Copy link
Contributor Author

Ok, I'll fix it;
Unfortunately I'm not a nodejs user and it could be a little bit difficult to test it properly

@bendi
Copy link
Owner

bendi commented Nov 15, 2016

Great!

Maybe you can use sample player in examples directory for testing?

@ntfshard
Copy link
Contributor Author

This patch should fix memory leak. Unfortunately I found same issues in a bendi/node-mp3info project =\

Happy holidays

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

No branches or pull requests

2 participants