Fix r14945: Fixed Write.cc:41 "!ccb->active()" assertion.

The r14945 patch has a major bug:
 When the Http::One::Server::writeControlMsgAndCall fails to write the control
message, schedules a Comm::Write callback using just a ScheduleCallHere command.
The callback called withtout the CommIoCbParams details and squid is crashes.

This patch fixes the ConnStateData::writeControlMsgAndCall to return false if it
fails to write the control message and allow the caller to handle the failure.

This is a Measurement Factory project

diff --git a/src/HttpControlMsg.cc b/src/HttpControlMsg.cc
index 1efc69b7ccdcbec75b7bcc7a4811a860fee1972c..29f9888a7e98fbe2ac40fd3b7010a84a002e462d 100644 (file)
--- a/src/HttpControlMsg.cc
+++ b/src/HttpControlMsg.cc
@@ -12,7 +12,7 @@
 #include "HttpControlMsg.h"
 
 void
-HttpControlMsgSink::wroteControlMsgOK()
+HttpControlMsgSink::doneWithControlMsg()
 {
     if (cbControlMsgSent) {
         ScheduleCallHere(cbControlMsgSent);
@@ -28,7 +28,7 @@ HttpControlMsgSink::wroteControlMsg(const CommIoCbParams &params)
         return;
 
     if (params.flag == Comm::OK) {
-        wroteControlMsgOK();
+        doneWithControlMsg();
         return;
     }
 

diff --git a/src/HttpControlMsg.h b/src/HttpControlMsg.h
index eb51dd3e044eaf592174d7c2ed7471ab1944aa05..cfeb53903bb5761a517c633ff3b9139a2c793cb7 100644 (file)
--- a/src/HttpControlMsg.h
+++ b/src/HttpControlMsg.h
@@ -33,7 +33,7 @@ public:
     /// called to send the 1xx message and notify the Source
     virtual void sendControlMsg(HttpControlMsg msg) = 0;
 
-    virtual void wroteControlMsgOK();
+    virtual void doneWithControlMsg();
 
     /// callback to handle Comm::Write completion
     void wroteControlMsg(const CommIoCbParams &);

diff --git a/src/client_side.cc b/src/client_side.cc
index ce3722645921a322f02ca3c2503e128fb1fe75ff..63803a1c06701ce74f94a532590d97ce461453cb 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -3825,7 +3825,10 @@ ConnStateData::sendControlMsg(HttpControlMsg msg)
         typedef CommCbMemFunT<HttpControlMsgSink, CommIoCbParams> Dialer;
         AsyncCall::Pointer call = JobCallback(33, 5, Dialer, this, HttpControlMsgSink::wroteControlMsg);
 
-        writeControlMsgAndCall(rep.getRaw(), call);
+        if (!writeControlMsgAndCall(rep.getRaw(), call)) {
+            // but still inform the caller (so it may resume its operation)
+            doneWithControlMsg();
+        }
         return;
     }
 
@@ -3834,9 +3837,9 @@ ConnStateData::sendControlMsg(HttpControlMsg msg)
 }
 
 void
-ConnStateData::wroteControlMsgOK()
+ConnStateData::doneWithControlMsg()
 {
-    HttpControlMsgSink::wroteControlMsgOK();
+    HttpControlMsgSink::doneWithControlMsg();
 
     if (Http::StreamPointer deferredRequest = pipeline.front()) {
         debugs(33, 3, clientConnection << ": calling PushDeferredIfNeeded after control msg wrote");

diff --git a/src/client_side.h b/src/client_side.h
index 7d32b16596ea64c39ddb5b51d59dfa20d70cb4b1..ba0d328e71a696cfbdaf6c6ebab3e56cbd66c0ca 100644 (file)
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -77,7 +77,7 @@ public:
 
     /* HttpControlMsgSink API */
     virtual void sendControlMsg(HttpControlMsg);
-    virtual void wroteControlMsgOK();
+    virtual void doneWithControlMsg();
 
     /// Traffic parsing
     bool clientParseRequests();
@@ -264,7 +264,7 @@ public:
     void connectionTag(const char *aTag) { connectionTag_ = aTag; }
 
     /// handle a control message received by context from a peer and call back
-    virtual void writeControlMsgAndCall(HttpReply *rep, AsyncCall::Pointer &call) = 0;
+    virtual bool writeControlMsgAndCall(HttpReply *rep, AsyncCall::Pointer &call) = 0;
 
     /// ClientStream calls this to supply response header (once) and data
     /// for the current Http::Stream.

diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc
index c30a1adb7a9dc600c593adefe032f5d78308d70f..42cbd0147d1420e9b6626eb9cc72baa0b474479d 100644 (file)
--- a/src/servers/FtpServer.cc
+++ b/src/servers/FtpServer.cc
@@ -1153,12 +1153,13 @@ Ftp::Server::writeForwardedForeign(const HttpReply *reply)
     writeErrorReply(reply, 451);
 }
 
-void
+bool
 Ftp::Server::writeControlMsgAndCall(HttpReply *reply, AsyncCall::Pointer &call)
 {
     // the caller guarantees that we are dealing with the current context only
     // the caller should also make sure reply->header.has(Http::HdrType::FTP_STATUS)
     writeForwardedReplyAndCall(reply, call);
+    return true;
 }
 
 void

diff --git a/src/servers/FtpServer.h b/src/servers/FtpServer.h
index f3025ba4112015ec00eabf1cfe84be0d0fe7d3fc..e44f7d5d3981382830a54d4aa12bf1a71f095fa7 100644 (file)
--- a/src/servers/FtpServer.h
+++ b/src/servers/FtpServer.h
@@ -97,7 +97,7 @@ protected:
     virtual void clientPinnedConnectionClosed(const CommCloseCbParams &io) override;
     virtual void handleReply(HttpReply *header, StoreIOBuffer receivedData) override;
     virtual int pipelinePrefetchMax() const override;
-    virtual void writeControlMsgAndCall(HttpReply *rep, AsyncCall::Pointer &call) override;
+    virtual bool writeControlMsgAndCall(HttpReply *rep, AsyncCall::Pointer &call) override;
     virtual time_t idleTimeout() const override;
 
     /* BodyPipe API */

diff --git a/src/servers/Http1Server.cc b/src/servers/Http1Server.cc
index fd5ef7796f8484e4b750ae158793fef15f4311d1..4129e0d4ceb1611d22ee0f763d2af0d838762434 100644 (file)
--- a/src/servers/Http1Server.cc
+++ b/src/servers/Http1Server.cc
@@ -306,7 +306,7 @@ Http::One::Server::handleReply(HttpReply *rep, StoreIOBuffer receivedData)
     context->sendStartOfMessage(rep, receivedData);
 }
 
-void
+bool
 Http::One::Server::writeControlMsgAndCall(HttpReply *rep, AsyncCall::Pointer &call)
 {
     Http::StreamPointer context = pipeline.front();
@@ -316,9 +316,7 @@ Http::One::Server::writeControlMsgAndCall(HttpReply *rep, AsyncCall::Pointer &ca
     // reply to the user already (e.g., after an error).
     if (context->reply) {
         debugs(11, 2, "drop 1xx made late by " << context->reply);
-        // but still inform the caller (so it may resume its operation)
-        ScheduleCallHere(call);
-        return;
+        return false;
     }
 
     const ClientHttpRequest *http = context->http;
@@ -337,6 +335,7 @@ Http::One::Server::writeControlMsgAndCall(HttpReply *rep, AsyncCall::Pointer &ca
     Comm::Write(clientConnection, mb, call);
 
     delete mb;
+    return true;
 }
 
 ConnStateData *

diff --git a/src/servers/Http1Server.h b/src/servers/Http1Server.h
index 4187a5c47d87482654694507749d6513c8d2f74c..d1cd50f865b29497b58d25634b210b96736e4718 100644 (file)
--- a/src/servers/Http1Server.h
+++ b/src/servers/Http1Server.h
@@ -32,7 +32,7 @@ protected:
     virtual Http::Stream *parseOneRequest();
     virtual void processParsedRequest(Http::StreamPointer &context);
     virtual void handleReply(HttpReply *rep, StoreIOBuffer receivedData);
-    virtual void writeControlMsgAndCall(HttpReply *rep, AsyncCall::Pointer &call);
+    virtual bool writeControlMsgAndCall(HttpReply *rep, AsyncCall::Pointer &call);
     virtual time_t idleTimeout() const;
 
     /* BodyPipe API */

diff --git a/src/tests/stub_HttpControlMsg.cc b/src/tests/stub_HttpControlMsg.cc
index ae01a9d138ea7023658208ce09c0c18d46cb5165..9fa482dc063a53ecdc0758ba0599987e7c7e905a 100644 (file)
--- a/src/tests/stub_HttpControlMsg.cc
+++ b/src/tests/stub_HttpControlMsg.cc
@@ -13,5 +13,5 @@
 
 #include "HttpControlMsg.h"
 void HttpControlMsgSink::wroteControlMsg(CommIoCbParams const&) STUB
-void HttpControlMsgSink::wroteControlMsgOK() STUB
+void HttpControlMsgSink::doneWithControlMsg() STUB