From: Amos Jeffries <squid3@treenet.co.nz>
Date: Sun, 23 May 2010 11:14:39 +0000 (+1200)
Subject: Author: Alex Rousskov <rousskov@measurement-factory.com>
X-Git-Tag: SQUID_3_1_4~20
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=38618511426feb5586f5ccffc553e18579234601;p=thirdparty%2Fsquid.git

Author: Alex Rousskov <rousskov@measurement-factory.com>
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 5f00e52295..52e0f94203 100644
--- a/src/Server.cc
+++ b/src/Server.cc
@@ -35,6 +35,7 @@
 #include "squid.h"
 #include "Server.h"
 #include "Store.h"
+#include "fde.h" /* for fd_table[fd].closing */
 #include "HttpRequest.h"
 #include "HttpReply.h"
 #include "TextException.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 171a6da0b3..78d869470d 100644
--- a/src/ftp.cc
+++ b/src/ftp.cc
@@ -1670,6 +1670,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 bcf2db9f00..c714e6f0bc 100644
--- a/src/http.cc
+++ b/src/http.cc
@@ -1979,6 +1979,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));
@@ -2081,6 +2088,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);