Re: MemPools rewrite

On 17 Nov 2000, at 18:42, Adrian Chadd <adrian@creative.net.au> wrote:

> On Fri, Nov 17, 2000, Andres Kroonmaa wrote:
> >
> > HEAD seems to leak memory. I'm testing chunked Pools on head, but my latest changes are
> > limited to other code, so I doubt I have added a bug.
> > look at "clientHttpRequest" and "cbdata" under Allocated
> > we shouldn't release 2.4 to the public until we are sure there isn't any leak.
> >
> > Current memory usage:
> > Pool Obj Size Chunks Allocated
>
> That is a classical "We've called lock/unlock wrongly" .. :)

 by diffing to 2.3 I think the culprit is here:

@@ -2427,7 +2581,8 @@
            for (H = &conn->chr; *H; H = &(*H)->next);
            *H = http;
            conn->nrequests++;
- commSetTimeout(fd, Config.Timeout.lifetime, NULL, NULL);
+ cbdataLock(http); /* lock for clientLifetimeTimeout() */
+ commSetTimeout(fd, Config.Timeout.lifetime, clientLifetimeTimeout, http);
            if (parser_return_code < 0) {
                debug(33, 1) ("clientReadRequest: FD %d Invalid Request\n", fd);
                err = errorCon(ERR_INVALID_REQ, HTTP_BAD_REQUEST);

@@ -2618,6 +2780,34 @@
...
...
+static void
+clientLifetimeTimeout(int fd, void *data)
+{
+ clientHttpRequest *http = data;
+ ConnStateData *conn;
+ int valid = cbdataValid(http);
+ cbdataUnlock(http);
+ if (!valid)
+ return;
+ conn = http->conn;
+ debug(33, 1) ("WARNING: Closing client %s connection due to lifetime timeout\n",
+ inet_ntoa(conn->peer.sin_addr));
+ debug(33, 1) ("\t%s\n", http->uri);
+ comm_close(fd);
 }

 Seems that we release http struct only if receive a client life timeout.
 I wonder why we need to lock http cbdata for this timeout at all?

------------------------------------
 Andres Kroonmaa <andre@online.ee>
 Delfi Online
 Tel: 6501 731, Fax: 6501 708
 Pärnu mnt. 158, Tallinn,
 11317 Estonia
Received on Fri Nov 17 2000 - 07:23:29 MST