Fix memory leak #94

Merged
kaykurokawa merged 2 commits from fix_memory_leak into master 2018-02-16 16:14:36 +01:00
kaykurokawa commented 2018-02-10 02:32:51 +01:00 (Migrated from github.com)

b2ba3f5
There was a memory leak which would cause a extra ClaimTrie node to be allocated but never deleted.
This would happen when a new node would be created in the ClaimTrie and CClaimTrieCache.addNodeToCache() was called.

The only reason the extra ClaimTrie node was created was to serve as a empty node template which would be copied by value over to the actual Node (I guess it looks like a DRY shortcut that was not implemented correctly). This is not necessary, and the fix just creates an empty node when necessary.

a2fa2c0
the was a missing clear() on a cache object in CClaimTrieCache. This should not cause any problem since we don't need to call clear() on map objects that are going to get destroyed anyways but nonetheless should be consistent.

Was able to reindex with this patch, and passes tests. Uses half the memory it used to according to my tests.

b2ba3f5 There was a memory leak which would cause a extra ClaimTrie node to be allocated but never deleted. This would happen when a new node would be created in the ClaimTrie and CClaimTrieCache.addNodeToCache() was called. The only reason the extra ClaimTrie node was created was to serve as a empty node template which would be copied by value over to the actual Node (I guess it looks like a DRY shortcut that was not implemented correctly). This is not necessary, and the fix just creates an empty node when necessary. a2fa2c0 the was a missing clear() on a cache object in CClaimTrieCache. This should not cause any problem since we don't need to call clear() on map objects that are going to get destroyed anyways but nonetheless should be consistent. Was able to reindex with this patch, and passes tests. Uses half the memory it used to according to my tests.
lyoshenka (Migrated from github.com) reviewed 2018-02-10 02:32:51 +01:00
kaykurokawa commented 2018-02-16 16:13:36 +01:00 (Migrated from github.com)

Steve has told me that he has reviewed the fix and tested it. So should be merging this.

Steve has told me that he has reviewed the fix and tested it. So should be merging this.
technovelist (Migrated from github.com) reviewed 2018-02-16 16:37:30 +01:00
technovelist (Migrated from github.com) left a comment

I have checked this under a heap checker and it fixes the leak of the CClaimTrieNode called "original" in this function.

I have checked this under a heap checker and it fixes the leak of the CClaimTrieNode called "original" in this function.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbrycrd#94
No description provided.