Do not call connect handler for closing descriptors because the handler

is unlikely to do something useful and is likely to hit Comm assertions
when working with a closing descriptor.

AFAIK, after adding close handlers to FtpStateData and peerProbe code,
all code that uses  commConnectStart has a Comm close or I/O handler that
will be called when the descriptor is closing. This should prevent
connecting jobs from getting stuck waiting for the connection callback
to be called.

diff --git a/src/CommCalls.cc b/src/CommCalls.cc
index 6e0ff4d9cc9563548dce1067fc73fa0626c91dec..6e5abbd154ac7cd4e8cd0e911da9f20bf872036e 100644 (file)
--- a/src/CommCalls.cc
+++ b/src/CommCalls.cc
@@ -55,6 +55,17 @@ CommConnectCbParams::CommConnectCbParams(void *aData):
 {
 }
 
+bool
+CommConnectCbParams::syncWithComm()
+{
+    // drop the call if the call was scheduled before comm_close but
+    // is being fired after comm_close
+    if (fd >= 0 && fd_table[fd].closing()) {
+       debugs(5, 3, HERE << "droppin late connect call: FD " << fd);
+        return false;
+    }
+    return true; // now we are in sync and can handle the call
+}
 
 /* CommIoCbParams */
 
@@ -63,7 +74,7 @@ CommIoCbParams::CommIoCbParams(void *aData): CommCommonCbParams(aData),
 {
 }
 
-void
+bool
 CommIoCbParams::syncWithComm()
 {
     // change parameters if the call was scheduled before comm_close but
@@ -73,6 +84,7 @@ CommIoCbParams::syncWithComm()
        flag = COMM_ERR_CLOSING;
        size = 0;
     }
+    return true; // now we are in sync and can handle the call
 }              
 
 

diff --git a/src/CommCalls.h b/src/CommCalls.h
index e7620508ce44c18270508600f3846d84f05ee07f..0a815072030151506517da91883f8b8446bdfd8c 100644 (file)
--- a/src/CommCalls.h
+++ b/src/CommCalls.h
@@ -41,8 +41,9 @@ public:
     CommCommonCbParams(const CommCommonCbParams &params);
     ~CommCommonCbParams();
 
-    /// last chance to adjust based on the current Comm state
-    void syncWithComm() {} // not virtual because fire() knows dialer type
+    /// adjust using the current Comm state; returns false to cancel the call
+    // not virtual because callers know dialer type
+    bool syncWithComm() { return true; } 
 
     void print(std::ostream &os) const;
 
@@ -73,6 +74,8 @@ public:
 class CommConnectCbParams: public CommCommonCbParams {
 public:
     CommConnectCbParams(void *aData);
+
+    bool syncWithComm(); // see CommCommonCbParams::syncWithComm
 };
 
 // read/write (I/O) parameters
@@ -81,7 +84,7 @@ public:
     CommIoCbParams(void *aData);
 
     void print(std::ostream &os) const;
-    void syncWithComm();
+    bool syncWithComm(); // see CommCommonCbParams::syncWithComm
 
 public:
     char *buf;
@@ -133,6 +136,9 @@ public:
     CommCbMemFunT(C *obj, Method meth): JobDialer(obj),
         CommDialerParamsT<Params>(obj), object(obj), method(meth) {}
 
+    virtual bool canDial(AsyncCall &c) { return JobDialer::canDial(c) && 
+        this->params.syncWithComm(); }
+
     virtual void print(std::ostream &os) const {
         os << '('; this->params.print(os); os << ')'; }
 
@@ -141,7 +147,7 @@ public:
     Method method;
 
 protected:
-    virtual void doDial() { this->params.syncWithComm(); (object->*method)(this->params); }
+    virtual void doDial() { (object->*method)(this->params); }
 };
 
 
@@ -282,6 +288,9 @@ CommCbFunPtrCallT<Dialer>::canFire()
     if (!cbdataReferenceValid(dialer.params.data))
         return cancel("callee gone");
 
+    if (!dialer.params.syncWithComm())
+        return cancel("out of sync w/comm");
+
     return true;
 }
 
@@ -289,7 +298,6 @@ template <class Dialer>
 void
 CommCbFunPtrCallT<Dialer>::fire()
 {
-    dialer.params.syncWithComm();
     dialer.dial();
 }