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 42d83334a76634179c2de5bbf76135658a8a75c9..b7c078178e79a530f7fdefc07734bf487622bfdb 100644 (file)
--- 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 8977a5b970cd409ea985c8faf0a46b55a311589e..8e3e156ca5cf9c1187457c7528258407c4f5a57d 100644 (file)
--- 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();