Squid calls cbdataFree() for objects that have non-POD data members such as refcounted pointers. Since cbdataFree() does not call the object destructor when freeing object memory, those data members are not properly destroyed, leading to leaks (at best). Some code tries to reset those pointers before calling cbdataFree(), but, naturally, there are quite a few places were we fail to cleanup. The other places are just ticking bombs until somebody adds a sensitive non-POD data member. TODO: replacee cbdataAlloc/Free() with new/delete? === modified file 'src/acl/Gadgets.cc' --- src/acl/Gadgets.cc 2012-07-02 12:28:10 +0000 +++ src/acl/Gadgets.cc 2012-07-07 05:13:12 +0000 @@ -260,41 +260,41 @@ ACLList *l; debugs(28, 8, "aclDestroyAclList: invoked"); for (l = *head; l; l = *head) { *head = l->next; delete l; } } void aclDestroyAccessList(acl_access ** list) { acl_access *l = NULL; acl_access *next = NULL; for (l = *list; l; l = next) { debugs(28, 3, "aclDestroyAccessList: '" << l->cfgline << "'"); next = l->next; aclDestroyAclList(&l->aclList); safe_free(l->cfgline); - cbdataFree(l); + delete l; } *list = NULL; } /* maex@space.net (06.09.1996) * destroy an acl_deny_info_list */ void aclDestroyDenyInfoList(acl_deny_info_list ** list) { acl_deny_info_list *a = NULL; acl_deny_info_list *a_next = NULL; acl_name_list *l = NULL; acl_name_list *l_next = NULL; debugs(28, 8, "aclDestroyDenyInfoList: invoked"); for (a = *list; a; a = a_next) { for (l = a->acl_list; l; l = l_next) { === modified file 'src/dns_internal.cc' --- src/dns_internal.cc 2012-03-05 11:36:38 +0000 +++ src/dns_internal.cc 2012-07-07 05:16:40 +0000 @@ -814,41 +814,41 @@ comm_read(conn, (char *)&vc->msglen, 2, call); vc->busy = 0; idnsDoSendQueryVC(vc); } static void idnsVCClosed(const CommCloseCbParams ¶ms) { nsvc * vc = (nsvc *)params.data; delete vc->queue; delete vc->msg; vc->conn = NULL; if (vc->ns < nns) // XXX: dnsShutdown may have freed nameservers[] nameservers[vc->ns].vc = NULL; cbdataFree(vc); } static void idnsInitVC(int ns) { - nsvc *vc = cbdataAlloc(nsvc); + nsvc *vc = cbdataAlloc(nsvc); // XXX: but has Comm::ConnectionPointer assert(ns < nns); assert(vc->conn == NULL); // MUST be NULL from the construction process! nameservers[ns].vc = vc; vc->ns = ns; vc->queue = new MemBuf; vc->msg = new MemBuf; vc->busy = 1; Comm::ConnectionPointer conn = new Comm::Connection(); if (!Config.Addrs.udp_outgoing.IsNoAddr()) conn->local = Config.Addrs.udp_outgoing; else conn->local = Config.Addrs.udp_incoming; conn->remote = nameservers[ns].S; if (conn->remote.IsIPv4()) { conn->local.SetIPv4(); } === modified file 'src/gopher.cc' --- src/gopher.cc 2012-01-20 18:55:04 +0000 +++ src/gopher.cc 2012-07-07 05:32:05 +0000 @@ -159,40 +159,41 @@ /// \ingroup ServerProtocolGopherInternal static char def_gopher_text[] = "text/plain"; /// \ingroup ServerProtocolGopherInternal static void gopherStateFree(const CommCloseCbParams ¶ms) { GopherStateData *gopherState = (GopherStateData *)params.data; if (gopherState == NULL) return; if (gopherState->entry) { gopherState->entry->unlock(); } HTTPMSGUNLOCK(gopherState->req); gopherState->fwd = NULL; // refcounted + gopherState->serverConn = NULL; // refcounted memFree(gopherState->buf, MEM_4K_BUF); gopherState->buf = NULL; cbdataFree(gopherState); } /** \ingroup ServerProtocolGopherInternal * Create MIME Header for Gopher Data */ static void gopherMimeCreate(GopherStateData * gopherState) { StoreEntry *entry = gopherState->entry; const char *mime_type = NULL; const char *mime_enc = NULL; switch (gopherState->type_id) { case GOPHER_DIRECTORY: @@ -939,41 +940,41 @@ debugs(10, 5, HERE << gopherState->serverConn); AsyncCall::Pointer call = commCbCall(5,5, "gopherSendComplete", CommIoCbPtrFun(gopherSendComplete, gopherState)); Comm::Write(gopherState->serverConn, buf, strlen(buf), call, NULL); if (EBIT_TEST(gopherState->entry->flags, ENTRY_CACHABLE)) gopherState->entry->setPublicKey(); /* Make it public */ } /// \ingroup ServerProtocolGopherInternal CBDATA_TYPE(GopherStateData); /// \ingroup ServerProtocolGopherAPI void gopherStart(FwdState * fwd) { StoreEntry *entry = fwd->entry; GopherStateData *gopherState; CBDATA_INIT_TYPE(GopherStateData); - gopherState = cbdataAlloc(GopherStateData); + gopherState = cbdataAlloc(GopherStateData); // XXX: but has Pointers gopherState->buf = (char *)memAllocate(MEM_4K_BUF); entry->lock(); gopherState->entry = entry; gopherState->fwd = fwd; debugs(10, 3, "gopherStart: " << entry->url() ); statCounter.server.all.requests++; statCounter.server.other.requests++; /* Parse url. */ gopher_request_parse(fwd->request, &gopherState->type_id, gopherState->request); comm_add_close_handler(fwd->serverConnection()->fd, gopherStateFree, gopherState); if (((gopherState->type_id == GOPHER_INDEX) || (gopherState->type_id == GOPHER_CSO)) === modified file 'src/helper.h' --- src/helper.h 2012-01-20 18:55:04 +0000 +++ src/helper.h 2012-07-07 04:52:46 +0000 @@ -53,41 +53,41 @@ public: wordlist *cmdline; dlink_list servers; dlink_list queue; const char *id_name; HelperChildConfig childs; ///< Configuration settings for number running. int ipc_type; Ip::Address addr; time_t last_queue_warn; time_t last_restart; char eom; ///< The char which marks the end of (response) message, normally '\n' struct _stats { int requests; int replies; int queue_size; int avg_svc_time; } stats; private: - CBDATA_CLASS2(helper); + CBDATA_CLASS2(helper); // XXX: but uses cbdataAlloc/cbdataFree }; class statefulhelper : public helper { public: inline statefulhelper(const char *name) : helper(name) {}; inline ~statefulhelper() {}; public: MemAllocator *datapool; HLPSAVAIL *IsAvailable; HLPSONEQ *OnEmptyQueue; private: CBDATA_CLASS2(statefulhelper); }; /* * Fields shared between stateless and stateful helper servers. */ === modified file 'src/ident/Ident.cc' --- src/ident/Ident.cc 2012-01-20 18:55:04 +0000 +++ src/ident/Ident.cc 2012-07-07 05:23:42 +0000 @@ -89,40 +89,41 @@ if (result && *result == '\0') result = NULL; while ((client = state->clients)) { void *cbdata; state->clients = client->next; if (cbdataReferenceValidDone(client->callback_data, &cbdata)) client->callback(result, cbdata); xfree(client); } } void Ident::Close(const CommCloseCbParams ¶ms) { IdentStateData *state = (IdentStateData *)params.data; identCallback(state, NULL); state->conn->close(); + state->conn = NULL; hash_remove_link(ident_hash, (hash_link *) state); xfree(state->hash.key); cbdataFree(state); } void Ident::Timeout(const CommTimeoutCbParams &io) { debugs(30, 3, HERE << io.conn); io.conn->close(); } void Ident::ConnectDone(const Comm::ConnectionPointer &conn, comm_err_t status, int xerrno, void *data) { IdentStateData *state = (IdentStateData *)data; if (status != COMM_OK) { if (status == COMM_TIMEOUT) { debugs(30, 3, "IDENT connection timeout to " << state->conn->remote); @@ -227,41 +228,41 @@ Ident::Start(const Comm::ConnectionPointer &conn, IDCB * callback, void *data) { IdentStateData *state; char key1[IDENT_KEY_SZ]; char key2[IDENT_KEY_SZ]; char key[IDENT_KEY_SZ]; conn->local.ToURL(key1, IDENT_KEY_SZ); conn->remote.ToURL(key2, IDENT_KEY_SZ); snprintf(key, IDENT_KEY_SZ, "%s,%s", key1, key2); if (!ident_hash) { Init(); } if ((state = (IdentStateData *)hash_lookup(ident_hash, key)) != NULL) { ClientAdd(state, callback, data); return; } CBDATA_INIT_TYPE(IdentStateData); - state = cbdataAlloc(IdentStateData); + state = cbdataAlloc(IdentStateData); // XXX: uses Comm::ConnectionPointer state->hash.key = xstrdup(key); // copy the conn details. We dont want the original FD to be re-used by IDENT. state->conn = conn->copyDetails(); // NP: use random port for secure outbound to IDENT_PORT state->conn->local.SetPort(0); ClientAdd(state, callback, data); hash_join(ident_hash, &state->hash); AsyncCall::Pointer call = commCbCall(30,3, "Ident::ConnectDone", CommConnectCbPtrFun(Ident::ConnectDone, state)); AsyncJob::Start(new Comm::ConnOpener(state->conn, call, Ident::TheConfig.timeout)); } void Ident::Init(void) { if (ident_hash) { debugs(30, DBG_CRITICAL, "WARNING: Ident already initialized."); return; === modified file 'src/neighbors.cc' --- src/neighbors.cc 2012-04-25 05:29:20 +0000 +++ src/neighbors.cc 2012-07-07 05:02:34 +0000 @@ -1407,41 +1407,41 @@ { ps_state *psstate = (ps_state *)data; StoreEntry *fake = psstate->entry; if (cbdataReferenceValid(psstate->callback_data)) { peer *p = (peer *)psstate->callback_data; p->mcast.flags.counting = 0; p->mcast.avg_n_members = Math::doubleAverage(p->mcast.avg_n_members, (double) psstate->ping.n_recv, ++p->mcast.n_times_counted, 10); debugs(15, 1, "Group " << p->host << ": " << psstate->ping.n_recv << " replies, "<< std::setw(4)<< std::setprecision(2) << p->mcast.avg_n_members <<" average, RTT " << p->stats.rtt); p->mcast.n_replies_expected = (int) p->mcast.avg_n_members; } cbdataReferenceDone(psstate->callback_data); fake->abort(); // sets ENTRY_ABORTED and initiates releated cleanup HTTPMSGUNLOCK(fake->mem_obj->request); fake->unlock(); HTTPMSGUNLOCK(psstate->request); - cbdataFree(psstate); + delete psstate; // XXX: but there is also peerSelectStateFree() } static void peerCountHandleIcpReply(peer * p, peer_t type, AnyP::ProtocolType proto, void *hdrnotused, void *data) { int rtt_av_factor; ps_state *psstate = (ps_state *)data; StoreEntry *fake = psstate->entry; MemObject *mem = fake->mem_obj; int rtt = tvSubMsec(mem->start_ping, current_time); assert(proto == AnyP::PROTO_ICP); assert(fake); assert(mem); psstate->ping.n_recv++; rtt_av_factor = RTT_AV_FACTOR; if (p->options.weighted_roundrobin) rtt_av_factor = RTT_BACKGROUND_AV_FACTOR; === modified file 'src/peer_select.cc' --- src/peer_select.cc 2012-05-08 01:21:10 +0000 +++ src/peer_select.cc 2012-07-07 04:58:18 +0000 @@ -90,41 +90,41 @@ eventDelete(peerPingTimeout, psstate); psstate->entry->ping_status = PING_DONE; } if (psstate->acl_checklist) { debugs(44, 1, "calling aclChecklistFree() from peerSelectStateFree"); delete (psstate->acl_checklist); } HTTPMSGUNLOCK(psstate->request); if (psstate->entry) { assert(psstate->entry->ping_status != PING_WAITING); psstate->entry->unlock(); psstate->entry = NULL; } delete psstate->lastError; - cbdataFree(psstate); + delete psstate; } static int peerSelectIcpPing(HttpRequest * request, int direct, StoreEntry * entry) { int n; assert(entry); assert(entry->ping_status == PING_NONE); assert(direct != DIRECT_YES); debugs(44, 3, "peerSelectIcpPing: " << entry->url() ); if (!request->flags.hierarchical && direct != DIRECT_NO) return 0; if (EBIT_TEST(entry->flags, KEY_PRIVATE) && !neighbors_do_private_keys) if (direct != DIRECT_NO) return 0; n = neighborsCount(request); === modified file 'src/stat.cc' --- src/stat.cc 2012-06-22 03:49:38 +0000 +++ src/stat.cc 2012-07-07 05:05:44 +0000 @@ -371,45 +371,45 @@ e->swap_dirn, e->swap_filen); if (mem != NULL) mem->stat (mb); mb->Printf("\n"); } /* process objects list */ static void statObjects(void *data) { StatObjectsState *state = static_cast(data); StoreEntry *e; if (state->theSearch->isDone()) { if (UsingSmp()) storeAppendPrintf(state->sentry, "} by kid%d\n\n", KidIdentifier); state->sentry->complete(); state->sentry->unlock(); - cbdataFree(state); + delete state; return; } else if (EBIT_TEST(state->sentry->flags, ENTRY_ABORTED)) { state->sentry->unlock(); - cbdataFree(state); + delete state; return; } else if (state->sentry->checkDeferRead(-1)) { state->sentry->flush(); eventAdd("statObjects", statObjects, state, 0.1, 1); return; } state->sentry->buffer(); size_t statCount = 0; MemBuf mb; mb.init(); while (statCount++ < static_cast(Config.Store.objectsPerBucket) && state-> theSearch->next()) { e = state->theSearch->currentItem(); if (state->filter && 0 == state->filter(e)) continue; statStoreEntry(&mb, e); === modified file 'src/store_swapout.cc' --- src/store_swapout.cc 2012-02-10 00:01:17 +0000 +++ src/store_swapout.cc 2012-07-07 05:09:04 +0000 @@ -289,41 +289,42 @@ StoreEntry::swapOutFileClose(int how) { assert(mem_obj != NULL); debugs(20, 3, "storeSwapOutFileClose: " << getMD5Text() << " how=" << how); debugs(20, 3, "storeSwapOutFileClose: sio = " << mem_obj->swapout.sio.getRaw()); if (mem_obj->swapout.sio == NULL) return; storeClose(mem_obj->swapout.sio, how); } static void storeSwapOutFileClosed(void *data, int errflag, StoreIOState::Pointer self) { generic_cbdata *c = (generic_cbdata *)data; StoreEntry *e = (StoreEntry *)c->data; MemObject *mem = e->mem_obj; assert(mem->swapout.sio == self); assert(e->swap_status == SWAPOUT_WRITING); - cbdataFree(c); + delete c; + c = NULL; // if object_size is still unknown, the entry was probably aborted if (errflag || e->objectLen() < 0) { debugs(20, 2, "storeSwapOutFileClosed: dirno " << e->swap_dirn << ", swapfile " << std::hex << std::setw(8) << std::setfill('0') << std::uppercase << e->swap_filen << ", errflag=" << errflag); if (errflag == DISK_NO_SPACE_LEFT) { /* FIXME: this should be handle by the link from store IO to * Store, rather than being a top level API call. */ e->store()->diskFull(); storeConfigure(); } if (e->swap_filen >= 0) e->unlink(); assert(e->swap_status == SWAPOUT_NONE); === modified file 'src/whois.cc' --- src/whois.cc 2012-01-20 18:55:04 +0000 +++ src/whois.cc 2012-07-07 05:25:44 +0000 @@ -69,41 +69,41 @@ CBDATA_TYPE(WhoisState); WhoisState::~WhoisState() { fwd = NULL; // refcounted } static void whoisWriteComplete(const Comm::ConnectionPointer &, char *buf, size_t size, comm_err_t flag, int xerrno, void *data) { xfree(buf); } void whoisStart(FwdState * fwd) { WhoisState *p; char *buf; size_t l; CBDATA_INIT_TYPE(WhoisState); - p = cbdataAlloc(WhoisState); + p = cbdataAlloc(WhoisState); // XXX: but uses FwdState::Pointer p->request = fwd->request; p->entry = fwd->entry; p->fwd = fwd; p->dataWritten = false; p->entry->lock(); comm_add_close_handler(fwd->serverConnection()->fd, whoisClose, p); l = p->request->urlpath.size() + 3; buf = (char *)xmalloc(l); String str_print=p->request->urlpath.substr(1,p->request->urlpath.size()); snprintf(buf, l, SQUIDSTRINGPH"\r\n", SQUIDSTRINGPRINT(str_print)); AsyncCall::Pointer writeCall = commCbCall(5,5, "whoisWriteComplete", CommIoCbPtrFun(whoisWriteComplete, p)); Comm::Write(fwd->serverConnection(), buf, strlen(buf), writeCall, NULL); AsyncCall::Pointer readCall = commCbCall(5,4, "whoisReadReply", CommIoCbPtrFun(whoisReadReply, p)); @@ -185,22 +185,23 @@ } /* no bytes read. stop reading */ entry->timestampsSet(); entry->flush(); if (!EBIT_TEST(entry->flags, RELEASE_REQUEST)) entry->setPublicKey(); fwd->complete(); debugs(75, 3, "whoisReadReply: Done: " << entry->url()); conn->close(); } static void whoisClose(const CommCloseCbParams ¶ms) { WhoisState *p = (WhoisState *)params.data; debugs(75, 3, "whoisClose: FD " << params.fd); p->entry->unlock(); + p->fwd = NULL; cbdataFree(p); }