From: Amos Jeffries Date: Sun, 24 Jan 2016 17:21:02 +0000 (+1300) Subject: Revert Pipeline ID logic changes X-Git-Tag: SQUID_4_0_5~14^2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2c39167619cb35d9f5e43f0575149998747f772b;p=thirdparty%2Fsquid.git Revert Pipeline ID logic changes --- diff --git a/src/Pipeline.cc b/src/Pipeline.cc index 58f2cc4448..32155f7c79 100644 --- 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 7515687a68..14d25d1427 100644 --- 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 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 51077963e6..de721fbe2f 100644 --- 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 d176e2bc2d..baeaa4959f 100644 --- 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 c29d33aa4d..dfc59917a0 100644 --- 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 05e5ccd317..08403306a0 100644 --- 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 00993a69ba..5b8f08cda2 100644 --- 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 ab4fe68e66..65bd30b6d4 100644 --- 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 21f8ff999a..e9bfbf3146 100644 --- 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 };