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 memleak and wrong make slice in firewall #42

Merged
merged 2 commits into from
Mar 14, 2022

Conversation

hirosumee
Copy link

No description provided.

@iamacarpet
Copy link
Owner

Thanks for the contribution @cuongvn98

We’ve used this code in production for quite a few years now and never noticed a memory leak or excessive memory usage (our app is a Windows Service that’s always running and querying info all the time).

Are you able to provide any explanation or evidence for why you feel this is necessary please?

If you’d like me to merge it, do you mind tidying it up and removing the files from your editor & the build tag that doesn’t match the rest of the repo please?

@iamacarpet
Copy link
Owner

Apologies @cuongvn98 ,

I can see something was reported by someone else a while ago:

#39

If you tidy things up I’ll be happy to merge.

I’ll have to look further into your helper functions for reading the properties, as while our own app doesn’t heavily use the firewall functions, we do frequently read a lot of WMI data elsewhere (and why I was confused we hadn’t noticed) so it might be useful to implement elsewhere.

Because missing release and clear memory after using.
@hirosumee
Copy link
Author

I will be remove unnecessary files soon
Evindences:
First look at the code , as u can see why enum , result of GetProperty function is not released after used . I will give you some evindences soon but you can reproduce memleak using following codes . You will see memory will be increase continous without release.

for {
   	_, _ = winapi.FirewallRulesGet()
   	time.Sleep(time.Second * 5)
      }

@hirosumee
Copy link
Author

I have delete unnessesary files

@iamacarpet iamacarpet merged commit ccb04d0 into iamacarpet:master Mar 14, 2022
@iamacarpet
Copy link
Owner

Thanks @cuongvn98

Looks good to me!

We aren’t actively using this part of the library, so if anyone notices any problems, please get in touch.

@hirosumee
Copy link
Author

Thank you so much !!!!

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.

2 participants