From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Sat, 22 May 2010 00:07:57 +0000 (-0600)
Subject: Bug 2896 fix: assertion failed: comm.cc:2063: "!fd_table[fd].closing()"
X-Git-Tag: SQUID_3_2_0_1~209
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a0297974162f56e0c8b35d6869c5574827bb9b3f;p=thirdparty%2Fsquid.git

Bug 2896 fix: assertion failed: comm.cc:2063: "!fd_table[fd].closing()"

When comm_close() has been called for the server fd but the close handler has
not yet been activated, the Server may receive an async call not associated
with the fd (e.g., more request body data coming from the HTTP or ICAP client)
that prompts the server to write to the fd.  We now check whether it is still
safe to write before writing. If it is not safe, we do not write but wait for
our close handler to be called.

TODO: when all comm_write callers check for fd closing, comm API can be
redefined to drop unsafe calls instead of asserting.
---

diff --git a/src/Server.cc b/src/Server.cc
index 1b7c820d23..6782881022 100644
--- a/src/Server.cc
+++ b/src/Server.cc
@@ -36,6 +36,7 @@
 #include "base/TextException.h"
 #include "Server.h"
 #include "Store.h"
+#include "fde.h" /* for fd_table[fd].closing */
 #include "HttpRequest.h"
 #include "HttpReply.h"
 #include "errorpage.h"
@@ -393,18 +394,32 @@ ServerStateData::sentRequestBody(const CommIoCbParams &io)
         sendMoreRequestBody();
 }
 
+bool
+ServerStateData::canSend(int fd) const
+{
+    return fd >= 0 && !fd_table[fd].closing();
+}
+
 void
 ServerStateData::sendMoreRequestBody()
 {
     assert(requestBodySource != NULL);
     assert(!requestSender);
+
+    const int fd = dataDescriptor();
+
+    if (!canSend(fd)) {
+        debugs(9,3, HERE << "cannot send request body to closing FD " << fd);
+        return; // wait for the kid's close handler; TODO: assert(closer);
+    }
+
     MemBuf buf;
     if (requestBodySource->getMoreData(buf)) {
         debugs(9,3, HERE << "will write " << buf.contentSize() << " request body bytes");
         typedef CommCbMemFunT<ServerStateData, CommIoCbParams> Dialer;
         requestSender = asyncCall(93,3, "ServerStateData::sentRequestBody",
                                   Dialer(this, &ServerStateData::sentRequestBody));
-        comm_write_mbuf(dataDescriptor(), &buf, requestSender);
+        comm_write_mbuf(fd, &buf, requestSender);
     } else {
         debugs(9,3, HERE << "will wait for more request body bytes or eof");
         requestSender = NULL;
diff --git a/src/Server.h b/src/Server.h
index cf6516ca14..27e722ab9d 100644
--- a/src/Server.h
+++ b/src/Server.h
@@ -128,6 +128,8 @@ protected:
     void handleRequestBodyProductionEnded();
     virtual void handleRequestBodyProducerAborted() = 0;
 
+    /// whether it is not too late to write to the server
+    bool canSend(int fd) const;
     // sending of the request body to the server
     void sendMoreRequestBody();
     // has body; kids overwrite to increment I/O stats counters
diff --git a/src/ftp.cc b/src/ftp.cc
index eff4ddc7f6..4cf5be5900 100644
--- a/src/ftp.cc
+++ b/src/ftp.cc
@@ -1521,6 +1521,12 @@ FtpStateData::writeCommand(const char *buf)
 
     ctrl.last_command = ebuf;
 
+    if (!canSend(ctrl.fd)) {
+        debugs(9, 2, HERE << "cannot send to closing ctrl FD " << ctrl.fd);
+        // TODO: assert(ctrl.closer != NULL);
+        return;
+    }
+
     typedef CommCbMemFunT<FtpStateData, CommIoCbParams> Dialer;
     AsyncCall::Pointer call = asyncCall(9, 5, "FtpStateData::ftpWriteCommandCallback",
                                         Dialer(this, &FtpStateData::ftpWriteCommandCallback));
diff --git a/src/http.cc b/src/http.cc
index d10bea993f..b5fd423df6 100644
--- a/src/http.cc
+++ b/src/http.cc
@@ -1992,6 +1992,13 @@ HttpStateData::sendRequest()
     MemBuf mb;
 
     debugs(11, 5, "httpSendRequest: FD " << fd << ", request " << request << ", this " << this << ".");
+
+    if (!canSend(fd)) {
+        debugs(11,3, HERE << "cannot send request to closing FD " << fd);
+        assert(closeHandler != NULL);
+        return false;
+    }
+
     typedef CommCbMemFunT<HttpStateData, CommTimeoutCbParams> TimeoutDialer;
     AsyncCall::Pointer timeoutCall =  asyncCall(11, 5, "HttpStateData::httpTimeout",
                                       TimeoutDialer(this,&HttpStateData::httpTimeout));
@@ -2095,6 +2102,13 @@ HttpStateData::doneSendingRequestBody()
             sendComplete(io);
         } else {
             debugs(11, 2, "doneSendingRequestBody: matched brokenPosts");
+
+            if (!canSend(fd)) {
+               debugs(11,2, HERE << "cannot send CRLF to closing FD " << fd);
+               assert(closeHandler != NULL);
+               return;
+            }
+
             typedef CommCbMemFunT<HttpStateData, CommIoCbParams> Dialer;
             Dialer dialer(this, &HttpStateData::sendComplete);
             AsyncCall::Pointer call= asyncCall(11,5, "HttpStateData::SendComplete", dialer);