-
Notifications
You must be signed in to change notification settings - Fork 728
fix: memory leak - remove circular reference in bind8 lib #528
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
base: master
Are you sure you want to change the base?
Conversation
|
I'm not so sure about this patch - it would mean that navigating down the named.conf parse tree and back up it wouldn't return to the same object. |
|
In worst case it must be rewritten. I tested it fast and it doesn't break anything, but something can count on this bug to work. This circular reference must be fixed, because leaked memory is huge ( around 12MB per domain restore ). In my opinion it doesn't rely on that mistake. EDIT: Inner key 'values' in 'parent' is still a reference. |
|
When you say "domain restore", are you talking about Virtualmin backups and restores? |
|
Yes |
|
Ok, what confuses me is that this patch doesn't seem related to any code that is invoked as part of the backup or restore process - if there is a memory leak, it should happen any time DNS records are listed or edited by Virtualmin. |
|
Ok, you can try it too. It's a problem on long running operations ( like the restore ). Ops like list/edit are living short time. |
|
I wasn't able to re-produce this - how large is the named.conf file on your system? |
|
1663 records, 258K |
|
So you have 1663 domains hosted by Virtualmin with DNS enabled? That's .... quite a lot. But I'm still having trouble re-producing this issue, or seeing where the memory leak could be coming from. Even though the parsed named.conf structure has a loop, this on it's own shouldn't cause a leak unless it is serialized - which doesn't happen anywhere in the backup / restore code. |
6ec1f01 to
75f0ca4
Compare
Spotted when restoring from backup.
So memory pressure when restoring is significantly reduced.