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 temp file path memory leak #3817

Merged
merged 2 commits into from Jan 2, 2020

Conversation

@stejbac
Copy link
Contributor

stejbac commented Dec 20, 2019

The internal java.io.DeleteOnExitHook uses a LinkedHashSet of paths to delete files at when the JVM exits. However, since there is no way to remove entries from the set, it leaks memory when calling deleteOnExit() for every new temp file created by FileManager.saveToFile. To avoid this, try to reuse temp file names (using one per FileManager instance).

FileManager.saveToFile appears to be called on average around every 2s, and this probably leaks around 36 + length * 2 bytes for the String object, 36 bytes for the hashtable node and maybe around 10 bytes for the table entry, or 82 + length * 2 ~= 236 bytes for a typical temp file path length on Windows - about 10MB per day.

stejbac added 2 commits Dec 20, 2019
As this private method is synchronized and not used in a way that
recursion/reentry is possible, any further instance-level locking within
the method is redundant - for each FileManager instance, there can only
be one saveToFile call in progress at a time.
The internal 'java.io.DeleteOnExitHook' uses a LinkedHashSet of paths to
to delete files at when the JVM exits. However, since there is no way to
remove entries from the set, it leaks memory when calling deleteOnExit()
for every new temp file created by FileManager.saveToFile. To avoid
this, try to reuse file names (using one per FileManager instance).

(FileManager.saveToFile appears to be called on average around every 2s,
and this probably leaks around 36 + length * 2 bytes for the String
object, 36 bytes for the hashtable node and maybe around 10 bytes for
the table entry, or 82 + length * 2 ~= 236 bytes for a typical temp file
path length on Windows - about 10MB per day.)
@stejbac stejbac requested a review from ripcurlx as a code owner Dec 20, 2019
Copy link
Contributor

chimp1984 left a comment

utACK

@chimp1984

This comment has been minimized.

Copy link
Contributor

chimp1984 commented Dec 21, 2019

Great finding!

@sqrrm
sqrrm approved these changes Jan 2, 2020
Copy link
Member

sqrrm left a comment

utACK

@sqrrm sqrrm merged commit ab3b0ec into bisq-network:master Jan 2, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ripcurlx ripcurlx added this to the v1.2.5 milestone Jan 2, 2020
@stejbac stejbac mentioned this pull request Jan 12, 2020
@sqrrm sqrrm mentioned this pull request Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.