Re^4: Tracing memory leak

Which looks fine, but only serves to tell us that what you've posted isn't indicative of your actual code.

AFAICT it is, but you can judge for yourself. There are two structures that get repeatedly created and destroyed. A connection object (this is a slightly unorthodox constructor since it is a class method of a base common to the client and server):

SV *acceptConnection (SV *objref) {
    char error[1024];
    HV *self = (HV*)SvRV(objref), *client = newHV();
    SV **field = hv_fetch(self, "fd", 2, 0);
    int srv = SvIV(*field), sock, flags, yes = 1;
    struct sockaddr_in info;
    socklen_t len = sizeof(struct sockaddr_in);

    sock = accept(srv, (struct sockaddr*)&info, &len);
    if (sock == -1) {
        snprintf(error, 1024, "accept(): %s", strerror(errno));
        hv_store(client, "sockerr", 7, newSVpv(error, 0), 0);
        close(sock);
    } else {
        setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE, &yes, sizeof(int));
        hv_store(client, "fd", 2, newSViv(sock), 0);
        hv_store(client, "ip", 2, newSVpv(inet_ntoa(info.sin_addr), 0)
+, 0);
        flags = fcntl(sock, F_GETFL, 0);
        if (flags == -1) {
            snprintf(error, 1024, "fcntl(F_GETFL): %s", strerror(errno
+));
            hv_store(client, "sockerr", 7, newSVpv(error, 0), 0);
            close(sock);
        } else if (fcntl(sock, F_SETFL, flags | O_NONBLOCK) == -1) {
            snprintf(error, 1024, "fcntl(F_SETFL O_NONBLOCK): %s", str
+error(errno));
            hv_store(client, "sockerr", 7, newSVpv(error, 0), 0);
            close(sock);
        } else {
            hv_store(client, "events", 6, newSViv(SPFD_READ), 0);
            hv_store(client, "state", 5, newSViv(READ_HEAD), 0);
            hv_store(client, "readerr", 7, newSViv(0), 0);
            hv_store(client, "errcount", 8, newSViv(0), 0);
            hv_store(client, "queue", 5, newRV_noinc((SV*)newAV()), 0)
+;
        }
    }

    return sv_bless(newRV_noinc((SV*)client), gv_stashpv("Sloop::Socke
+t", 0));
}
[download]

And a request hash, produced by "parseRequest()" which contains a hash produced by "parseHeaders()":

HV *parseHeaders (char **data) {
    HV *hdrs = newHV();
    char copy[8192], *p, *val = NULL, *str = *data;
    int state = NAME;
    SV **cur;

    while (1) {

    // 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;
        }

    // extract field value
        if (!val) {
            p = val = strchr(str, ':');
            val++;
            while (*val == ' ' || *val == '\t') val++;
        }

        if (!(*p)) break;    // malformed
        if (p[0] == '\r' && p[1] == '\n') {
            p += 2;
            if (*p > 32 || (p[0] == '\r' && p[1] == '\n')) {
            // next line starts with a new name or \r\n, so field is c
+omplete
                *(p-2) = '\0';
                if (SvCUR(*cur)) {    // multiple entries
                    val--;
                    *val = ',';
                }

                sv_catpv(*cur, val);
                if (p[0] == '\r' && p[1] == '\n') break;    // good
                val = NULL;
                state = NAME;
                str = p;
            }
        } else p++;
    }

    *data = p;
    return hdrs;
}

HV *parseRequest (char *raw, int stop) {
    int len, state = METHOD;
    char *p = raw, *val, *qs;
    SV **field;
    HV *rv = newHV();

    hv_store(rv, "err", 3, newSViv(INCOMPLETE), 0);

    while (p - raw < stop) {


    // HTTP 1.x Request Line extracted in METHOD->PATH->PROTOCOL state
+s
        if (state == METHOD) {
            while (*p == ' ' || *p == '\t') p++;
            val = p;
            while (*p != ' ' && *p != '\t' && *p != '\0') p++;
            *p = '\0';
            lowerCase(val);
            hv_store(rv, "method", 6, newSVpv(val, 0), 0);
            p++;
            state = PATH;
            continue;    // loop condition checks bounds ;)
        }

        if (state == PATH) {
            while (*p <= 32 && *p > 0) p++;
            if (*p != '/' && *p != '*') {
            // must be an absolute path or * (RFC 2616)
                field = hv_fetch(rv, "err", 3, 0);
                sv_setiv(*field, BAD_PATH);
                break;
            }
        // turn path into an array
            AV *path = newAV();
            val = p+1;
            if (*val <= 32) {
                av_push(path, newSVpv(p, 1));
                p++;
            } else {
                while (*p > 32) p++;
                *p = '\0';
            // check for query string
                if ((qs = strrchr(val, '?'))) {
                    *qs = '\0';
                    qs = queryString(rv, ++qs);
                }
                while ((p = strchr(val, '/'))) {
                    if (!*(p+1)) break;// path ends with '/'
                    av_push(path, newSVpv(val, p-val));
                    val = ++p;
                }
                len = strlen(val);
                if (qs) p = qs + 1;
                else p = val + len + 1;
                if (val[len-1] == '/') len--;
                av_push(path, newSVpv(val, len));
            }
            hv_store(rv, "path", 4, newRV_noinc((SV*)path), 0);
            state = PROTOCOL;
            continue;
        }

        if (state == PROTOCOL) {
            while (*p == ' ' || *p == '\t') p++;
            if (*p < 32)    // no protocol
                hv_store(rv, "protocol", 8, newSVpv("???", 0), 0);
            else {
                val = p;
                while (*p > 32) p++;
                *(p++) = '\0';
                hv_store(rv, "protocol", 8, newSVpv(val, 0), 0);
            }
            while (*p <= 32 && *p) p++;
            break;
        }
    }

    if (state >= PROTOCOL) {

        hv_store(rv, "headers", 7, newRV_noinc((SV*)parseHeaders(&p)),
+ 0);
        while (*p == '\r' || *p == '\n') p++;

        if (*p && p < raw + stop) hv_store(rv, "body", 4, newSVpv(p, s
+top - (p-raw)), 0);
        field = hv_fetch(rv, "err", 3, 0);
        sv_setiv(*field, OK);
    }

    return rv;
}
[download]

Comment on Re^4: Tracing memory leak Select or Download Code

Replies are listed 'Best First'.
Re^5: Tracing memory leak by BrowserUk (Patriarch) on Sep 15, 2011 at 00:56 UTC
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.	[reply]
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]
Re^7: Tracing memory leak by BrowserUk (Patriarch) on Sep 15, 2011 at 11:50 UTC
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]


XP is just a number
	PerlMonks