From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Sun, 28 Sep 2008 14:48:27 +0000 (-0600)
Subject: Fixed comm_close handling in deferred reads. The code was expecting old-style
X-Git-Tag: SQUID_3_1_0_1~45^2~26
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2796c0d7c8ff0a22fb151446bd190bf675aa994f;p=thirdparty%2Fsquid.git

Fixed comm_close handling in deferred reads. The code was expecting old-style
comm_remove_close_handler call to work if the close handler has not been
dialed yet. We now store a new-style callback so that we can reliably cancel
the close hander call.

Removed all methods from CommRead except for constructors. Apparently,
they were all unused and most were not even defined.
---

diff --git a/src/CommRead.h b/src/CommRead.h
index 42d83334a7..b7c078178e 100644
--- a/src/CommRead.h
+++ b/src/CommRead.h
@@ -51,19 +51,10 @@ class CommRead
 public:
     CommRead ();
     CommRead (int fd, char *buf, int len, AsyncCall::Pointer &callback);
-    void queueCallback(size_t retval, comm_err_t errcode, int xerrno);
-    bool hasCallback() const;
-    void hasCallbackInvariant() const;
-    void hasNoCallbackInvariant() const;
-    void tryReading();
-    void read();
-    void initiateActualRead();
-    void doCallback(comm_err_t errcode, int xerrno);
     int fd;
     char *buf;
     int len;
     AsyncCall::Pointer callback;
-    static void ReadTry(int fd, void *data);
 };
 
 class DeferredRead
@@ -78,6 +69,7 @@ public:
     void *theContext;
     CommRead theRead;
     bool cancelled;
+    AsyncCall::Pointer closer; ///< internal close handler used by Comm
 
 private:
 };
diff --git a/src/comm.cc b/src/comm.cc
index 8977a5b970..8e3e156ca5 100644
--- a/src/comm.cc
+++ b/src/comm.cc
@@ -1499,21 +1499,6 @@ comm_reset_close(int fd)
     comm_close(fd);
 }
 
-void
-CommRead::doCallback(comm_err_t errcode, int xerrno)
-{
-    if (callback != NULL) {
-        typedef CommIoCbParams Params;
-        Params &params = GetCommParams<Params>(callback);
-        params.fd = fd;
-        params.size = 0;
-        params.flag = errcode;
-        params.xerrno = xerrno;
-        ScheduleCallHere(callback);
-        callback = NULL;
-    }
-}
-
 void 
 comm_close_start(int fd, void *data)
 {
@@ -2519,7 +2504,15 @@ void
 DeferredReadManager::delayRead(DeferredRead const &aRead) {
     debugs(5, 3, "Adding deferred read on FD " << aRead.theRead.fd);
     CbDataList<DeferredRead> *temp = deferredReads.push_back(aRead);
-    comm_add_close_handler (aRead.theRead.fd, CloseHandler, temp);
+
+    // We have to use a global function as a closer and point to temp 
+    // instead of "this" because DeferredReadManager is not a job and
+    // is not even cbdata protected
+    AsyncCall::Pointer closer = commCbCall(5,4,
+        "DeferredReadManager::CloseHandler",
+        CommCloseCbPtrFun(&CloseHandler, temp));
+    comm_add_close_handler(aRead.theRead.fd, closer);
+    temp->element.closer = closer; // remeber so that we can cancel
 }
 
 void
@@ -2529,6 +2522,7 @@ DeferredReadManager::CloseHandler(int fd, void *thecbdata) {
 
     CbDataList<DeferredRead> *temp = (CbDataList<DeferredRead> *)thecbdata;
 
+    temp->element.closer = NULL;
     temp->element.markCancelled();
 }
 
@@ -2536,8 +2530,11 @@ DeferredRead
 DeferredReadManager::popHead(CbDataListContainer<DeferredRead> &deferredReads) {
     assert (!deferredReads.empty());
 
-    if (!deferredReads.head->element.cancelled)
-        comm_remove_close_handler(deferredReads.head->element.theRead.fd, CloseHandler, deferredReads.head);
+    DeferredRead &read = deferredReads.head->element;
+    if (!read.cancelled) {
+        comm_remove_close_handler(read.theRead.fd, read.closer);
+        read.closer = NULL;
+    }
 
     DeferredRead result = deferredReads.pop_front();