Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#643 closed defect (fixed)

Memory leaks in the cache

Reported by: vorner Owned by: zhanglikun
Priority: medium Milestone: R-Team-Sprint-20110316
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: 8.0 Add Hours to Ticket: 0
Total Hours: 16.0 Internal?: no

Description

Valgrind on the tests in #614 says there are memory leaks around the cache. It seems to me most of the reports are from ResolverCache::update.

It should be examined and fixed.

Subtickets

Attachments (1)

cache.valgrind (9.2 KB) - added by vorner 9 years ago.
Output of valgrind

Download all attachments as: .zip

Change History (12)

comment:1 Changed 50 years ago by zhanglikun

  • Add Hours to Ticket changed from 0.0 to 16.0
  • Total Hours changed from 0.0 to 16.0

Changed 9 years ago by vorner

Output of valgrind

comment:1 Changed 9 years ago by shane

  • Milestone changed from R-Team-Task-Backlog to R-Team-Sprint-20110308

comment:2 Changed 9 years ago by jelte

  • Owner set to jelte
  • Status changed from new to assigned

comment:3 Changed 9 years ago by jelte

Likun mentioned that these should be fixed by #661. Since I'm running a derivative of another branch I can't tell yet whether this is the case outside of the unit tests, so I'm keeping this ticket open for now. If #661 does indeed fix it, I'll mark this one as a duplicate.

comment:4 Changed 9 years ago by zhanglikun

  • Status changed from assigned to reviewing

Hi Jelte, would you like to have a look at the latest commit? I tested in my local machine, no leak now.

comment:5 Changed 9 years ago by zhanglikun

  • Owner changed from jelte to zhanglikun

oh, wait....
when I run the valgrind again today, it reports a lot of memory leak, which it didn't give last friday. (don't know the reason, does it try to give me a good weekend?)

comment:6 Changed 9 years ago by zhanglikun

oh, yes, it doesn't have memory leak in cache, (last comment was tested in the wrong branch 661, sorry for the noise), I will try to fix the leak problem in ticket 641

comment:7 Changed 9 years ago by zhanglikun

  • Owner changed from zhanglikun to jelte

This ticket is ready for review.

comment:8 follow-up: Changed 9 years ago by jelte

  • Owner changed from jelte to zhanglikun

Only one slight comment, the 'left the size of the list 0' seems a bit off; should this be something like "When done, the size of the list will be 0"? (it occurs at least twice in lru_list.h).

Oh and just a question (i think the answer is yes), is it safe to call the drop handler from the lru_list (are we sure we cannot do this while the entry is somehow still in the hash table?)

If the answer is yes, and you clarify the comment, I think this is good and can be merged (i don't see any valgrind problems anymore in cache/tests)

comment:9 in reply to: ↑ 8 Changed 9 years ago by zhanglikun

  • Resolution set to fixed
  • Status changed from reviewing to closed

Replying to jelte:

Only one slight comment, the 'left the size of the list 0' seems a bit off; should this be something like "When done, the size of the list will be 0"? (it occurs at least twice in lru_list.h).

Done, Thanks

Oh and just a question (i think the answer is yes), is it safe to call the drop handler from the lru_list (are we sure we cannot do this while the entry is somehow still in the hash table?)

Yes, it's safe, since the drop handler is tranferred from hash table, currently lru list and hash table are seperate, there is one ticket about merging them together. When one entry is removed from lru list, it has to be removed from hash table, or else, the entry will never be deleted(also we have no way to iterate hash table).

If the answer is yes, and you clarify the comment, I think this is good and can be merged (i don't see any valgrind problems anymore in cache/tests)

Done, thanks, see git aba4c4067da0dc63c97c6356dc3137651755ffce.

comment:10 Changed 9 years ago by zhanglikun

  • Add Hours to Ticket changed from 0 to 16
  • Estimated Difficulty changed from 0.0 to 8
Note: See TracTickets for help on using tickets.