From: adrian <>
Date: Wed, 18 Feb 2004 08:58:59 +0000 (+0000)
Subject: * create a new method in CallBack which does the cbdataReferenceValid()
X-Git-Tag: SQUID_3_0_PRE4~1138
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=85a56f1517fb49c3d31f7207f90f135a74a9bf9d;p=thirdparty%2Fsquid.git

* create a new method in CallBack which does the cbdataReferenceValid()
  for us
* remove the extra cbdataReference/cbdataReferenceDone() in the comm
  connect codepath: this was exposing a bug with how cbdataReferenceDone()
  operates. In a nutshell, its a macro which sets the value to NULL after
  decrementing its refcount because, after that function call, the memory
  may have been freed. The problem was that we were doing it on the data
  pointer in a CallBack instance and so the instance never had a chance to
  remove _its_ reference count on the data because, at the time the
  destructor is called, the data pointer has been made NULL.

  Not nice.

* Fix the comm connect finish routine to properly get rid of any CallBack
  which may be allocated. cbdataReferenceDone() would NOT have been
  sufficient if the connection had timed out - we'd still have been left
  with an extra refcount without the above modifications.
---

diff --git a/src/CommRead.h b/src/CommRead.h
index 5c93a377f8..73cebbc03d 100644
--- a/src/CommRead.h
+++ b/src/CommRead.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: CommRead.h,v 1.5 2003/07/10 09:37:56 robertc Exp $
+ * $Id: CommRead.h,v 1.6 2004/02/18 01:58:59 adrian Exp $
  *
  * DEBUG: section 5    Comms
  * AUTHOR: Robert Collins <robertc@squid-cache.org>
@@ -79,6 +79,10 @@ public:
         return *this;
     }
 
+    bool dataValid()
+    {
+        return cbdataReferenceValid(data);
+    }
 
     bool operator == (CallBack const &rhs) { return handler==rhs.handler && data==rhs.data;}
 
diff --git a/src/comm.cc b/src/comm.cc
index 76db40b1c7..efbe2064b2 100644
--- a/src/comm.cc
+++ b/src/comm.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: comm.cc,v 1.391 2003/10/20 12:33:01 robertc Exp $
+ * $Id: comm.cc,v 1.392 2004/02/18 01:58:59 adrian Exp $
  *
  * DEBUG: section 5     Socket Functions
  * AUTHOR: Harvest Derived
@@ -1302,7 +1302,7 @@ commConnectStart(int fd, const char *host, u_short port, CNCB * callback, void *
     cs->fd = fd;
     cs->host = xstrdup(host);
     cs->port = port;
-    cs->callback = CallBack<CNCB>(callback,cbdataReference(data));
+    cs->callback = CallBack<CNCB>(callback, data);
     comm_add_close_handler(fd, commConnectFree, cs);
     cs->locks++;
     ipcache_nbgethostbyname(host, commConnectDnsHandle, cs);
@@ -1345,11 +1345,9 @@ ConnectStateData::callCallback(comm_err_t status, int xerrno)
     callback = CallBack<CNCB>();
     commSetTimeout(fd, -1, NULL, NULL);
 
-    if (cbdataReferenceValid(aCallback.data))
+    if (aCallback.dataValid())
         aCallback.handler(fd, status, xerrno, aCallback.data);
 
-    cbdataReferenceDone(aCallback.data);
-
     commConnectFree(fd, this);
 }
 
@@ -1358,7 +1356,7 @@ commConnectFree(int fd, void *data)
 {
     ConnectStateData *cs = (ConnectStateData *)data;
     debug(5, 3) ("commConnectFree: FD %d\n", fd);
-    cbdataReferenceDone(cs->callback.data);
+    cs->callback = CallBack<CNCB>();
     safe_free(cs->host);
     delete cs;
 }