ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-332

c client issues (memory leaks) reported by valgrind

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Invalid
    • Affects Version/s: 3.1.0
    • Fix Version/s: 3.1.1, 3.2.0
    • Component/s: c client
    • Labels:
      None

      Description

      Attaching valgrind log files.

      1) getpwuid_r doesn't seem like it's due to us
      2) the rest seem to be valid

      1. valgrind_mt.out
        5 kB
        Patrick Hunt
      2. valgrind_st.out
        5 kB
        Patrick Hunt

        Activity

        Hide
        Patrick Hunt added a comment -

        btw, ran valgrind like:

        valgrind --malloc-fill=aa --free-fill=bb --log-file=valgrind_mt.out --leak-check=yes ./zktest-mt

        Show
        Patrick Hunt added a comment - btw, ran valgrind like: valgrind --malloc-fill=aa --free-fill=bb --log-file=valgrind_mt.out --leak-check=yes ./zktest-mt
        Hide
        Chris Darroch added a comment -

        These look OK to me, on a first glance. There's a call to freeaddrinfo() in getaddrs() to match getaddrinfo(). There's also a free(zh) in zookeeper_close() to match the allocation in zookeeper_init(). I think these are just spurious reports from valgrind, but I'm not absolutely 100% on that.

        Show
        Chris Darroch added a comment - These look OK to me, on a first glance. There's a call to freeaddrinfo() in getaddrs() to match getaddrinfo(). There's also a free(zh) in zookeeper_close() to match the allocation in zookeeper_init(). I think these are just spurious reports from valgrind, but I'm not absolutely 100% on that.
        Hide
        Mahadev konar added a comment -

        I agree with chris. also for the create_completion_entry( zookeeper.c:1830 in zktest-st) ( the line number is different in the latest code after a few checkins)... has a matcing free() in process_completions. I think we can close this jira for now.

        Show
        Mahadev konar added a comment - I agree with chris. also for the create_completion_entry( zookeeper.c:1830 in zktest-st) ( the line number is different in the latest code after a few checkins)... has a matcing free() in process_completions. I think we can close this jira for now.
        Hide
        Patrick Hunt added a comment -

        I did do a quick eyeball of the results before entering the jira - I guess the "goto fail" in getaddrs caught my eye - that skips the freeaddrinfo call, but now looking again it seems thats an unlikely case (oom).

        Feel free to close, it might be good to document this somewhere, perhaps a src/c/valgrind.txt file that details the exceptions in case someone else runs valgrind? Isn't there some way to pass known exceptions to valgrind? Perhaps that's a better way (commit the exceptions to svn).

        anyway - looks like we are good for 3.1.1, feel free to close.

        Show
        Patrick Hunt added a comment - I did do a quick eyeball of the results before entering the jira - I guess the "goto fail" in getaddrs caught my eye - that skips the freeaddrinfo call, but now looking again it seems thats an unlikely case (oom). Feel free to close, it might be good to document this somewhere, perhaps a src/c/valgrind.txt file that details the exceptions in case someone else runs valgrind? Isn't there some way to pass known exceptions to valgrind? Perhaps that's a better way (commit the exceptions to svn). anyway - looks like we are good for 3.1.1, feel free to close.
        Hide
        Patrick Hunt added a comment -

        perhaps a new jira to update the makefile to include valgrind/helgrind support? it can load the exceptions that we know about explicitly... just a thought.

        Show
        Patrick Hunt added a comment - perhaps a new jira to update the makefile to include valgrind/helgrind support? it can load the exceptions that we know about explicitly... just a thought.
        Hide
        Mahadev konar added a comment -

        i dont think their is anything like passing known exceptions to valgrind. Also, the exceptions are line number based, which would be difficult to track with code changes in the c code. I would just close this jira for now and revisit it again if someone opens another jira for this. On the other hand, it would be good to have hudson scream if the number of excpetions increase...

        Show
        Mahadev konar added a comment - i dont think their is anything like passing known exceptions to valgrind. Also, the exceptions are line number based, which would be difficult to track with code changes in the c code. I would just close this jira for now and revisit it again if someone opens another jira for this. On the other hand, it would be good to have hudson scream if the number of excpetions increase...
        Hide
        Mahadev konar added a comment -

        I am resolving this for now.

        Show
        Mahadev konar added a comment - I am resolving this for now.
        Hide
        Patrick Hunt added a comment -

        FYI:

        --suppressions=<filename> [default: $PREFIX/lib/valgrind/default.supp]
        Specifies an extra file from which to read descriptions of
        errors to suppress. You may use up to 100 extra suppression
        files.

        also:

        --gen-suppressions=<yes|no|all> [default: no]

        Show
        Patrick Hunt added a comment - FYI: --suppressions=<filename> [default: $PREFIX/lib/valgrind/default.supp] Specifies an extra file from which to read descriptions of errors to suppress. You may use up to 100 extra suppression files. also: --gen-suppressions=<yes|no|all> [default: no]

          People

          • Assignee:
            Unassigned
            Reporter:
            Patrick Hunt
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development