Fixed ConnOpener callback's syncWithComm()

The stale CommConnectCbParams override was testing unused (i.e. always
negative) CommConnectCbParams::fd and was trying to cancel the callback
that most (possibly all) recipients rely on: ConnOpener users expect a
negative answer rather than no answer at all.

diff --git a/src/CommCalls.cc b/src/CommCalls.cc
index 3becd0aead19f24f43450ee48090eb8888a42067..c18bc4c6d601d58a3744af0cd1a9f4933ee62315 100644 (file)
--- a/src/CommCalls.cc
+++ b/src/CommCalls.cc
@@ -53,6 +53,9 @@ CommAcceptCbParams::CommAcceptCbParams(void *aData):
 {
 }
 
+// XXX: Add CommAcceptCbParams::syncWithComm(). Adjust syncWithComm() API if all
+// implementations always return true.
+
 void
 CommAcceptCbParams::print(std::ostream &os) const
 {
@@ -72,13 +75,24 @@ 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 << "dropping late connect call: FD " << fd);
-        return false;
+    assert(conn);
+
+    // change parameters if this is a successful callback that was scheduled
+    // after its Comm-registered connection started to close
+
+    if (flag != Comm::OK) {
+        assert(!conn->isOpen());
+        return true; // not a successful callback; cannot go out of sync
     }
-    return true; // now we are in sync and can handle the call
+
+    assert(conn->isOpen());
+    if (!fd_table[conn->fd].closing())
+        return true; // no closing in progress; in sync (for now)
+
+    debugs(5, 3, "converting to Comm::ERR_CLOSING: " << conn);
+    conn->noteClosure();
+    flag = Comm::ERR_CLOSING;
+    return true; // now the callback is in sync with Comm again
 }
 
 /* CommIoCbParams */

diff --git a/src/comm/ConnOpener.cc b/src/comm/ConnOpener.cc
index 5625f16065eed87c2958eca0d8b18b63c0b2a0d2..6c7032b6ebf91991ea9ff29173b32c1672542e49 100644 (file)
--- a/src/comm/ConnOpener.cc
+++ b/src/comm/ConnOpener.cc
@@ -143,9 +143,6 @@ Comm::ConnOpener::sendAnswer(Comm::Flag errFlag, int xerrno, const char *why)
             else
                 assert(conn_->isOpen());
 
-            // XXX: CommConnectCbParams imply syncWithComm(). Our recipients
-            // need a callback even if conn is closing. TODO: Do not reuse
-            // CommConnectCbParams; implement a different syncing logic.
             typedef CommConnectCbParams Params;
             Params &params = GetCommParams<Params>(callback_);
             params.conn = conn_;

diff --git a/src/tests/stub_libcomm.cc b/src/tests/stub_libcomm.cc
index 022a7e8242e89701b91a752c74a0e4afa46b1a1b..3d4d32743eb2d4f8bd78138ea61f1809c0916b6f 100644 (file)
--- a/src/tests/stub_libcomm.cc
+++ b/src/tests/stub_libcomm.cc
@@ -25,6 +25,7 @@ Comm::Connection::~Connection() STUB
 Comm::ConnectionPointer Comm::Connection::cloneIdentDetails() const STUB_RETVAL(nullptr)
 Comm::ConnectionPointer Comm::Connection::cloneDestinationDetails() const STUB_RETVAL(nullptr)
 void Comm::Connection::close() STUB
+void Comm::Connection::noteClosure() STUB
 CachePeer * Comm::Connection::getPeer() const STUB_RETVAL(NULL)
 void Comm::Connection::setPeer(CachePeer * p) STUB
 ScopedId Comm::Connection::codeContextGist() const STUB_RETVAL(id.detach())