Improved handling of closing-while-in-callback connections

Needs more work, but at least same-callback handlers are (more) similar
now, illustrating the kind of callback reactions the future
unified/automated API will have to support.

diff --git a/src/FwdState.cc b/src/FwdState.cc
index c02b5833bfa30125b28b9415e6cfc186ea0951eb..38865abcf11bc5f74e997c8d785f65fdf95427d8 100644 (file)
--- a/src/FwdState.cc
+++ b/src/FwdState.cc
@@ -834,6 +834,8 @@ FwdState::noteConnection(HappyConnOpener::Answer &answer)
         Must(!Comm::IsConnOpen(answer.conn));
         answer.error.clear(); // preserve error for errorSendComplete()
     } else if (!Comm::IsConnOpen(answer.conn) || fd_table[answer.conn->fd].closing()) {
+        // The socket could get closed while our callback was queued. Sync
+        // Connection. XXX: Connection::fd may already be stale/invalid here.
         // We do not know exactly why the connection got closed, so we play it
         // safe, allowing retries only for persistent (reused) connections
         if (answer.reused) {
@@ -919,8 +921,8 @@ FwdState::tunnelEstablishmentDone(Http::TunnelerAnswer &answer)
         Must(error);
         answer.squidError.clear(); // preserve error for fail()
     } else if (!Comm::IsConnOpen(answer.conn) || fd_table[answer.conn->fd].closing()) {
-        // The socket could get closed while our callback was queued.
-        // We close Connection here to sync Connection::fd.
+        // The socket could get closed while our callback was queued. Sync
+        // Connection. XXX: Connection::fd may already be stale/invalid here.
         closePendingConnection(answer.conn, "conn was closed while waiting for tunnelEstablishmentDone");
         error = new ErrorState(ERR_CANNOT_FORWARD, Http::scServiceUnavailable, request, al);
     } else if (!answer.leftovers.isEmpty()) {
@@ -1012,6 +1014,8 @@ FwdState::connectedToPeer(Security::EncryptorAnswer &answer)
         complete(); // destroys us
         return;
     } else if (!Comm::IsConnOpen(answer.conn) || fd_table[answer.conn->fd].closing()) {
+        // The socket could get closed while our callback was queued. Sync
+        // Connection. XXX: Connection::fd may already be stale/invalid here.
         closePendingConnection(answer.conn, "conn was closed while waiting for connectedToPeer");
         error = new ErrorState(ERR_CANNOT_FORWARD, Http::scServiceUnavailable, request, al);
     }

diff --git a/src/PeerPoolMgr.cc b/src/PeerPoolMgr.cc
index 9e0bf1b4d0347c1c6e921a9feba06469c9e8fd4a..fb12c9c0e8a9b227541baa08a48aa54f7db7b8f6 100644 (file)
--- a/src/PeerPoolMgr.cc
+++ b/src/PeerPoolMgr.cc
@@ -153,7 +153,15 @@ PeerPoolMgr::handleSecuredPeer(Security::EncryptorAnswer &answer)
         return;
     }
 
-    // TODO: Check for .closing() like FwdState::connectedToPeer() does?
+    assert(answer.conn);
+
+    // The socket could get closed while our callback was queued. Sync
+    // Connection. XXX: Connection::fd may already be stale/invalid here.
+    if (answer.conn->isOpen() && fd_table[answer.conn->fd].closing()) {
+        answer.conn->noteClosure();
+        checkpoint("external connection closure"); // may retry
+        return;
+    }
 
     pushNewConnection(answer.conn);
 }

diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc
index 6e4cf75ab4d183644084f31023f8ff05570b3266..0b84b7208ef03e718f94629a16551d6359380f88 100644 (file)
--- a/src/adaptation/icap/Xaction.cc
+++ b/src/adaptation/icap/Xaction.cc
@@ -727,8 +727,17 @@ Adaptation::Icap::Xaction::handleSecuredPeer(Security::EncryptorAnswer &answer)
 
     debugs(93, 5, "TLS negotiation to " << service().cfg().uri << " complete");
 
-    // XXX: answer.conn could be closing here. Missing a syncWithComm equivalent?
-    // TODO: Check for .closing() like FwdState::connectedToPeer() does?
+    assert(answer.conn);
+
+    // The socket could get closed while our callback was queued. Sync
+    // Connection. XXX: Connection::fd may already be stale/invalid here.
+    if (answer.conn->isOpen() && fd_table[answer.conn->fd].closing()) {
+        answer.conn->noteClosure();
+        service().noteConnectionFailed("external TLS connection closure");
+        detailError(ERR_DETAIL_ICAP_XACT_SSL_START);
+        throw TexcHere("external closure of the TLS ICAP service connection");
+    }
+
     useIcapConnection(answer.conn);
 }