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 &params)
 {
     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 &params)
 {
     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 &params)
 {
     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<StatObjectsState *>(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<size_t>(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 &params)
 {
     WhoisState *p = (WhoisState *)params.data;
     debugs(75, 3, "whoisClose: FD " << params.fd);
     p->entry->unlock();
+    p->fwd = NULL;
     cbdataFree(p);
 }


