diff --git a/squid-5.5-store-client-leak-fix.patch b/squid-5.5-store-client-leak-fix.patch new file mode 100644 index 0000000..536397f --- /dev/null +++ b/squid-5.5-store-client-leak-fix.patch @@ -0,0 +1,163 @@ +diff --git a/src/StoreClient.h b/src/StoreClient.h +index 484b413..b523cc0 100644 +--- a/src/StoreClient.h ++++ b/src/StoreClient.h +@@ -203,7 +203,7 @@ private: + public: + + struct Callback { +- Callback ():callback_handler(NULL), callback_data(NULL) {} ++ Callback() = default; + + Callback (STCB *, void *); + +@@ -212,8 +212,8 @@ public: + /// delivery to the STCB callback_handler. + bool pending() const; + +- STCB *callback_handler; +- void *callback_data; ++ STCB *callback_handler = nullptr; ///< where to deliver the answer ++ CallbackData cbData; ///< the first STCB callback parameter + CodeContextPointer codeContext; ///< Store client context + + /// a scheduled asynchronous finishCallback() call (or nil) +diff --git a/src/store_client.cc b/src/store_client.cc +index d8ae028..e7144b4 100644 +--- a/src/store_client.cc ++++ b/src/store_client.cc +@@ -188,14 +188,12 @@ store_client::finishCallback() + ++answers; + + STCB *temphandler = _callback.callback_handler; +- void *cbdata = _callback.callback_data; +- _callback = Callback(NULL, NULL); ++ const auto cbdata = _callback.cbData.validDone(); ++ _callback = Callback(); + copyInto.data = NULL; + +- if (cbdataReferenceValid(cbdata)) ++ if (cbdata) + temphandler(cbdata, result); +- +- cbdataReferenceDone(cbdata); + } + + store_client::store_client(StoreEntry *e) : +@@ -255,7 +253,7 @@ store_client::copy(StoreEntry * anEntry, + #endif + + assert(!_callback.pending()); +- _callback = Callback (callback_fn, cbdataReference(data)); ++ _callback = Callback(callback_fn, data); + copyInto.data = copyRequest.data; + copyInto.length = copyRequest.length; + copyInto.offset = copyRequest.offset; +@@ -1129,7 +1127,7 @@ store_client::dumpStats(MemBuf * output, int clientNumber) const + if (_callback.pending()) + return; + +- output->appendf("\tClient #%d, %p\n", clientNumber, _callback.callback_data); ++ output->appendf("\tClient #%d, %p\n", clientNumber, this); + output->appendf("\t\tcopy_offset: %" PRId64 "\n", copyInto.offset); + output->appendf("\t\tcopy_size: %" PRIuSIZE "\n", copyInto.length); + output->append("\t\tflags:", 8); +@@ -1154,7 +1152,7 @@ store_client::Callback::pending() const + + store_client::Callback::Callback(STCB *function, void *data): + callback_handler(function), +- callback_data(data), ++ cbData(data), + codeContext(CodeContext::Current()) + { + } +diff --git a/src/tunnel.cc b/src/tunnel.cc +index 4fc5abd..2d37d2e 100644 +--- a/src/tunnel.cc ++++ b/src/tunnel.cc +@@ -273,6 +273,7 @@ private: + /// resumes operations after the (possibly failed) HTTP CONNECT exchange + void tunnelEstablishmentDone(Http::TunnelerAnswer &answer); + ++ void finishWritingAndDelete(Connection &); + void deleteThis(); + + void cancelStep(const char *reason); +@@ -319,7 +320,9 @@ TunnelStateData::serverClosed() + { + server.noteClosure(); + +- retryOrBail(__FUNCTION__); ++ request->hier.stopPeerClock(false); ++ ++ finishWritingAndDelete(client); + } + + /// TunnelStateData::clientClosed() wrapper +@@ -334,12 +337,48 @@ void + TunnelStateData::clientClosed() + { + client.noteClosure(); ++ finishWritingAndDelete(server); ++} + ++/// Gracefully handles non-retriable connection closure. If necessary, either ++/// starts closing the given connection or waits for its pending write to ++/// finish. Otherwise, immediately destroys the tunnel object. ++/// \prec The other Connection is not open. ++void ++TunnelStateData::finishWritingAndDelete(Connection &remainingConnection) ++{ + if (noConnections()) + return deleteThis(); + +- if (!server.writer) +- server.conn->close(); ++ // XXX: The code below should precede the noConnections() check above. When ++ // there is no writer, we should trigger an immediate noConnections() ++ // outcome instead of waiting for an asynchronous call to our own closure ++ // callback (that will call this method again). We should not move this code ++ // until a noConnections() outcome guarantees a nil writer because such a ++ // move will unnecessary delay deleteThis(). ++ ++ if (remainingConnection.writer) { ++ debugs(26, 5, "waiting to finish writing to " << remainingConnection.conn); ++ // the write completion callback must close its remainingConnection ++ // after noticing that the other connection is gone ++ return; ++ } ++ ++ // XXX: Stop abusing connection closure callback for terminating tunneling ++ // in cases like this, where our code knows that tunneling must end. The ++ // closure callback should be dedicated to handling rare connection closures ++ // originated _outside_ of TunnelStateData (e.g., during shutdown). In all ++ // other cases, our own close()-calling code must detail the ++ // closure-triggering error (if any) _and_ clear all callbacks: Our code ++ // does not need to be (asynchronously) notified of the closure that it ++ // itself has initiated! Until that (significant) refactoring, ++ // serverClosed() and clientClosed() callbacks will continue to mishandle ++ // those rare closures as regular ones, and access.log records will continue ++ // to lack some tunneling error indicators/details. ++ // ++ // This asynchronous close() leads to another finishWritingAndDelete() call ++ // but with true noConnections() that finally triggers deleteThis(). ++ remainingConnection.conn->close(); + } + + /// destroys the tunnel (after performing potentially-throwing cleanup) +@@ -451,14 +490,7 @@ TunnelStateData::retryOrBail(const char *context) + return sendError(savedError, bailDescription ? bailDescription : context); + *status_ptr = savedError->httpStatus; + +- if (noConnections()) +- return deleteThis(); +- +- // This is a "Comm::IsConnOpen(client.conn) but !canSendError" case. +- // Closing the connection (after finishing writing) is the best we can do. +- if (!client.writer) +- client.conn->close(); +- // else writeClientDone() must notice a closed server and close the client ++ finishWritingAndDelete(client); + } + + int diff --git a/squid.spec b/squid.spec index 0511097..58de82a 100644 --- a/squid.spec +++ b/squid.spec @@ -2,7 +2,7 @@ Name: squid Version: 5.5 -Release: 20%{?dist} +Release: 21%{?dist} Summary: The Squid proxy caching server Epoch: 7 # See CREDITS for breakdown of non GPLv2+ code @@ -54,6 +54,8 @@ Patch211: squid-5.5-large-upload-buffer-dies.patch Patch212: squid-5.5-fatal-read-data-from-mem.patch # https://issues.redhat.com/browse/RHEL-77282 Patch213: squid-5.5-cache-peer-connect-errors.patch +# https://issues.redhat.com/browse/RHEL-50261 +Patch214: squid-5.5-store-client-leak-fix.patch # Security patches # https://bugzilla.redhat.com/show_bug.cgi?id=2100721 @@ -185,6 +187,8 @@ lookup program (dnsserver), a program for retrieving FTP data # patch506 follow-up %patch212 -p1 -b .fatal-read-data-from-mem +%patch214 -p1 -b .store-client-mem-leak + # https://bugzilla.redhat.com/show_bug.cgi?id=1679526 # Patch in the vendor documentation and used different location for documentation sed -i 's|@SYSCONFDIR@/squid.conf.documented|%{_pkgdocdir}/squid.conf.documented|' src/squid.8.in @@ -410,6 +414,9 @@ fi %changelog +* Wed Jul 16 2025 Luboš Uhliarik - 7:5.5-21 +- Resolves: RHEL-50261 - Squid memory usage increases until OOM. + * Thu Apr 10 2025 Luboš Uhliarik - 7:5.5-20 - Resolves: RHEL-77282 - ”TCP connection to XX.XX.XX.XX/XXXX failed” message is output frequently on RHEL9