* 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 5c93a377f8105f3fe24378b11903f23009b8f66f..73cebbc03d43f98ef1e94a1ae6848dd8b424dea8 100644 (file)
--- 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 76db40b1c733a5cd505c688620150dc6ceacf7ab..efbe2064b2a49923bcc1814bf20c0b8a0b000f23 100644 (file)
--- 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;
 }