Revert Pipeline ID logic changes

diff --git a/src/Pipeline.cc b/src/Pipeline.cc
index 58f2cc44487168632278a56dccc5ccae0bd97c33..32155f7c79f732710ce01d9627a7ce205d4896cf 100644 (file)
--- a/src/Pipeline.cc
+++ b/src/Pipeline.cc
@@ -21,7 +21,6 @@ Pipeline::add(const Http::StreamContextPointer &c)
 {
     requests.push_back(c);
     ++nrequests;
-    ++nactive;
     debugs(33, 3, "Pipeline " << (void*)this << " add request " << nrequests << ' ' << c);
 }
 
@@ -50,24 +49,15 @@ Pipeline::terminateAll(int xerrno)
 }
 
 void
-Pipeline::popById(uint32_t which)
+Pipeline::popMe(const Http::StreamContextPointer &which)
 {
     if (requests.empty())
         return;
 
-    debugs(33, 3, "Pipeline " << (void*)this << " drop id=" << which);
-
-    // find the context and clear its Pointer
-    for (auto &&i : requests) {
-        if (i->id == which) {
-            i = nullptr;
-            --nactive;
-            break;
-        }
-    }
-
-    // trim closed contexts from the list head (if any)
-    while (!requests.empty() && !requests.front())
-        requests.pop_front();
+    debugs(33, 3, "Pipeline " << (void*)this << " drop " << requests.front());
+    // in reality there may be multiple contexts doing processing in parallel.
+    // XXX: pipeline still assumes HTTP/1 FIFO semantics are obeyed.
+    assert(which == requests.front());
+    requests.pop_front();
 }
 

diff --git a/src/Pipeline.h b/src/Pipeline.h
index 7515687a6884a485f2428cec3e4e9c9f483415c4..14d25d14278ba6ded43ea1590906ac5f41790bde 100644 (file)
--- a/src/Pipeline.h
+++ b/src/Pipeline.h
@@ -37,7 +37,7 @@ class Pipeline
     Pipeline & operator =(const Pipeline &) = delete;
 
 public:
-    Pipeline() : nrequests(0), nactive(0) {}
+    Pipeline() : nrequests(0) {}
     ~Pipeline() = default;
 
     /// register a new request context to the pipeline
@@ -47,7 +47,7 @@ public:
     Http::StreamContextPointer front() const;
 
     /// how many requests are currently pipelined
-    size_t count() const {return nactive;}
+    size_t count() const {return requests.size();}
 
     /// whether there are none or any requests currently pipelined
     bool empty() const {return requests.empty();}
@@ -55,8 +55,8 @@ public:
     /// tell everybody about the err, and abort all waiting requests
     void terminateAll(const int xerrno);
 
-    /// deregister a request from the pipeline
-    void popById(uint32_t);
+    /// deregister the front request from the pipeline
+    void popMe(const Http::StreamContextPointer &);
 
     /// Number of requests seen in this pipeline (so far).
     /// Includes incomplete transactions.
@@ -65,10 +65,6 @@ public:
 private:
     /// requests parsed from the connection but not yet completed.
     std::list<Http::StreamContextPointer> requests;
-
-    /// Number of still-active streams in this pipeline (so far).
-    /// Includes incomplete transactions.
-    uint32_t nactive;
 };
 
 #endif /* SQUID_SRC_PIPELINE_H */

diff --git a/src/client_side.cc b/src/client_side.cc
index 51077963e6360664df6f11655a130560be51f77d..de721fbe2ff0645e1c74f94b7c81bd132dc36321 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -1014,7 +1014,7 @@ ConnStateData::abortRequestParsing(const char *const uri)
     http->req_sz = inBuf.length();
     http->uri = xstrdup(uri);
     setLogUri (http, uri);
-    auto *context = new Http::StreamContext(nextStreamId(), clientConnection, http);
+    auto *context = new Http::StreamContext(clientConnection, http);
     StoreIOBuffer tempBuffer;
     tempBuffer.data = context->reqbuf;
     tempBuffer.length = HTTP_REQBUF_SZ;
@@ -1360,7 +1360,7 @@ parseHttpRequest(ConnStateData *csd, const Http1::RequestParserPointer &hp)
     ClientHttpRequest *http = new ClientHttpRequest(csd);
 
     http->req_sz = hp->messageHeaderSize();
-    Http::StreamContext *result = new Http::StreamContext(csd->nextStreamId(), csd->clientConnection, http);
+    Http::StreamContext *result = new Http::StreamContext(csd->clientConnection, http);
 
     StoreIOBuffer tempBuffer;
     tempBuffer.data = result->reqbuf;
@@ -1577,7 +1577,8 @@ clientTunnelOnError(ConnStateData *conn, Http::StreamContext *context, HttpReque
                 // XXX: Either the context is finished() or it should stay queued.
                 // The below may leak client streams BodyPipe objects. BUT, we need
                 // to check if client-streams detatch is safe to do here (finished() will detatch).
-                conn->pipeline.popById(context->id);
+                assert(conn->pipeline.front() == context); // XXX: still assumes HTTP/1 semantics
+                conn->pipeline.popMe(Http::StreamContextPointer(context));
             }
             Comm::SetSelect(conn->clientConnection->fd, COMM_SELECT_READ, NULL, NULL, 0);
             conn->fakeAConnectRequest("unknown-protocol", conn->preservedClientData);

diff --git a/src/client_side.h b/src/client_side.h
index d176e2bc2de81eaaf4bef8391dced17e30a8dcf5..baeaa4959f166b649897f71a8e27433dcd283ae0 100644 (file)
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -69,7 +69,6 @@ public:
     virtual ~ConnStateData();
 
     /* ::Server API */
-    virtual uint32_t nextStreamId() {return ++nextStreamId_;}
     virtual void receivedFirstByte();
     virtual bool handleReadData();
     virtual void afterClientRead();

diff --git a/src/http/StreamContext.cc b/src/http/StreamContext.cc
index c29d33aa4ddb48085b8ffe8091d7dd56228ca508..dfc59917a06e5a4046ebdf41b169665b0588369c 100644 (file)
--- a/src/http/StreamContext.cc
+++ b/src/http/StreamContext.cc
@@ -14,8 +14,7 @@
 #include "Store.h"
 #include "TimeOrTag.h"
 
-Http::StreamContext::StreamContext(uint32_t anId, const Comm::ConnectionPointer &aConn, ClientHttpRequest *aReq) :
-    id(anId),
+Http::StreamContext::StreamContext(const Comm::ConnectionPointer &aConn, ClientHttpRequest *aReq) :
     clientConnection(aConn),
     http(aReq),
     reply(nullptr),
@@ -547,7 +546,7 @@ Http::StreamContext::finished()
 
     assert(connRegistered_);
     connRegistered_ = false;
-    conn->pipeline.popById(id);
+    conn->pipeline.popMe(Http::StreamContextPointer(this));
 }
 
 /// called when we encounter a response-related error

diff --git a/src/http/StreamContext.h b/src/http/StreamContext.h
index 05e5ccd317e8791ee3a1352b4413f6f2214743ad..08403306a055ea67e96fb4a7f99bb5a0ac812558 100644 (file)
--- a/src/http/StreamContext.h
+++ b/src/http/StreamContext.h
@@ -68,7 +68,7 @@ class StreamContext : public RefCountable
 
 public:
     /// construct with HTTP/1.x details
-    StreamContext(uint32_t id, const Comm::ConnectionPointer &, ClientHttpRequest *);
+    StreamContext(const Comm::ConnectionPointer &aConn, ClientHttpRequest *aReq);
     ~StreamContext();
 
     /// register this stream with the Server
@@ -119,10 +119,6 @@ public:
 
     void deferRecipientForLater(clientStreamNode *, HttpReply *, StoreIOBuffer receivedData);
 
-public:
-    // NP: stream ID is relative to the connection, not global.
-    uint32_t id; ///< stream ID within the client connection.
-
 public: // HTTP/1.x state data
 
     Comm::ConnectionPointer clientConnection; ///< details about the client connection socket

diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc
index 00993a69ba19de2336c90e6910a046995eb7ade8..5b8f08cda21c0cb0e26cabdb505e62240ea63899 100644 (file)
--- a/src/servers/FtpServer.cc
+++ b/src/servers/FtpServer.cc
@@ -742,7 +742,7 @@ Ftp::Server::parseOneRequest()
     http->uri = newUri;
 
     Http::StreamContext *const result =
-        new Http::StreamContext(nextStreamId(), clientConnection, http);
+        new Http::StreamContext(clientConnection, http);
 
     StoreIOBuffer tempBuffer;
     tempBuffer.data = result->reqbuf;

diff --git a/src/servers/Server.cc b/src/servers/Server.cc
index ab4fe68e6690da126aec9b2c6bb7dfd86b581112..65bd30b6d41329bb798c99b9bc2159451dcefbf8 100644 (file)
--- a/src/servers/Server.cc
+++ b/src/servers/Server.cc
@@ -26,8 +26,7 @@ Server::Server(const MasterXaction::Pointer &xact) :
     clientConnection(xact->tcpClient),
     transferProtocol(xact->squidPort->transport),
     port(xact->squidPort),
-    receivedFirstByte_(false),
-    nextStreamId_(0)
+    receivedFirstByte_(false)
 {}
 
 bool

diff --git a/src/servers/Server.h b/src/servers/Server.h
index 21f8ff999a57b604fe8c0a43afa5c13811ef0908..e9bfbf314635d0e4291dd82dc1aa1b0292f567aa 100644 (file)
--- a/src/servers/Server.h
+++ b/src/servers/Server.h
@@ -35,9 +35,6 @@ public:
     virtual bool doneAll() const;
     virtual void swanSong();
 
-    /// fetch the next available stream ID
-    virtual uint32_t nextStreamId() = 0;
-
     /// ??
     virtual bool connFinishedWithConn(int size) = 0;
 
@@ -120,7 +117,6 @@ protected:
     void doClientRead(const CommIoCbParams &io);
     void clientWriteDone(const CommIoCbParams &io);
 
-    uint32_t nextStreamId_;    ///< incremented as streams are initiated
     AsyncCall::Pointer reader; ///< set when we are reading
     AsyncCall::Pointer writer; ///< set when we are writing
 };