in reply to Re^4: Tracing memory leak
in thread Tracing memory leak

There is too much missing from what you've posted to be able to mock something up to allow it to compile.

The most productive technique I've found for tracking down such leaks is a simple, if rather laborious one of commenting out most of the body of the code -- for example, so that it returns just an empty hash or even just a scalar -- and checking that it no longer leaks. Then put it back in chunks until the leak reappears. Not sophisticated, but reliable.

Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
"Science is about questioning the status quo. Questioning authority".
In the absence of evidence, opinion is indistinguishable from prejudice.

Replies are listed 'Best First'.
Re^6: Tracing memory leak
by halfcountplus (Hermit) on Sep 15, 2011 at 11:40 UTC
    It was in the parseHeaders() routine from the previous post. It is kind of obvious now, I must have placed my faith in some impossible API voodoo: 
        // extract field name
        if (state == NAME) {
            if (sscanf(str, "%[^:]:", copy) != 1) break;    // malform
+ed
            lowerCase(copy);
            cur = hv_fetch(hdrs, copy, strlen(copy), 1);
            if (!SvOK(*cur)) *cur = newSVpv("", 0);
            state = VALUE;
            continue;
        }

    [download]
    Setting the last arg to hv_fetch will create a key-value pair if it does not exist already and return a pointer to the value pointer, a newSV(0) (ie, undef, so !SvOK means the key is new). Since I wanted to concat multiple entries for the same header field into a CSV string, I reassigned a newSVpv("") to the pointer pointer, leaking the undef SV it pointed to. Changing to this: 
                cur = hv_fetch(hdrs, copy, strlen(copy), 1);
            if (!SvOK(*cur)) {
                SvREFCNT_dec(*cur);
                *cur = newSVpv("", 0);
            }

    [download]

    Fixed the leak...probably the reason I didn't see this issue at first is because the API has no "free" functions, it relies on reference counts. But I put my money on voodoo (if you aren't sure what to do, don't do anything, lol).

    I found this by checking PL_sv_count before and after function calls. parseHeaders() returns a hash of headers, but after the call PL_sv_count was incremented by 2 (one for the hash, one for the ref it is assigned to in parseRequest()) + twice as many keys as were actually in the hash.

[reply]
[d/l]
[select]

      Well done for finding it and many thanks for reporting back the source. Way too few people do that.

      I spent quite a while starring at your code, including that bit, but I still didn't see it.

      And many thanks to dave_the_m for suggesting the use of PL_sv_count. I can see that being extremely useful in the future now we know about it. I shall be adding it to some of my debug macros forthwith.

      Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
      "Science is about questioning the status quo. Questioning authority".
      In the absence of evidence, opinion is indistinguishable from prejudice.
[reply]
[d/l]

In Section Seekers of Perl Wisdom