193 lines
7.8 KiB
Diff
193 lines
7.8 KiB
Diff
|
diff --git a/src/http.cc b/src/http.cc
|
||
|
index 98e3969..8b55bf3 100644
|
||
|
--- a/src/http.cc
|
||
|
+++ b/src/http.cc
|
||
|
@@ -54,6 +54,7 @@
|
||
|
#include "rfc1738.h"
|
||
|
#include "SquidConfig.h"
|
||
|
#include "SquidTime.h"
|
||
|
+#include "SquidMath.h"
|
||
|
#include "StatCounters.h"
|
||
|
#include "Store.h"
|
||
|
#include "StrList.h"
|
||
|
@@ -1235,18 +1236,26 @@ HttpStateData::readReply(const CommIoCbParams &io)
|
||
|
* Plus, it breaks our lame *HalfClosed() detection
|
||
|
*/
|
||
|
|
||
|
- Must(maybeMakeSpaceAvailable(true));
|
||
|
- CommIoCbParams rd(this); // will be expanded with ReadNow results
|
||
|
- rd.conn = io.conn;
|
||
|
- rd.size = entry->bytesWanted(Range<size_t>(0, inBuf.spaceSize()));
|
||
|
+ const auto moreDataPermission = canBufferMoreReplyBytes();
|
||
|
+ if (!moreDataPermission) {
|
||
|
+ abortTransaction("ready to read required data, but the read buffer is full and cannot be drained");
|
||
|
+ return;
|
||
|
+ }
|
||
|
+
|
||
|
+ const auto readSizeMax = maybeMakeSpaceAvailable(moreDataPermission.value());
|
||
|
+ // TODO: Move this logic inside maybeMakeSpaceAvailable():
|
||
|
+ const auto readSizeWanted = readSizeMax ? entry->bytesWanted(Range<size_t>(0, readSizeMax)) : 0;
|
||
|
|
||
|
- if (rd.size <= 0) {
|
||
|
+ if (readSizeWanted <= 0) {
|
||
|
assert(entry->mem_obj);
|
||
|
AsyncCall::Pointer nilCall;
|
||
|
entry->mem_obj->delayRead(DeferredRead(readDelayed, this, CommRead(io.conn, NULL, 0, nilCall)));
|
||
|
return;
|
||
|
}
|
||
|
|
||
|
+ CommIoCbParams rd(this); // will be expanded with ReadNow results
|
||
|
+ rd.conn = io.conn;
|
||
|
+ rd.size = readSizeWanted;
|
||
|
switch (Comm::ReadNow(rd, inBuf)) {
|
||
|
case Comm::INPROGRESS:
|
||
|
if (inBuf.isEmpty())
|
||
|
@@ -1617,8 +1626,10 @@ HttpStateData::maybeReadVirginBody()
|
||
|
if (!Comm::IsConnOpen(serverConnection) || fd_table[serverConnection->fd].closing())
|
||
|
return;
|
||
|
|
||
|
- if (!maybeMakeSpaceAvailable(false))
|
||
|
+ if (!canBufferMoreReplyBytes()) {
|
||
|
+ abortTransaction("more response bytes required, but the read buffer is full and cannot be drained");
|
||
|
return;
|
||
|
+ }
|
||
|
|
||
|
// XXX: get rid of the do_next_read flag
|
||
|
// check for the proper reasons preventing read(2)
|
||
|
@@ -1636,40 +1647,78 @@ HttpStateData::maybeReadVirginBody()
|
||
|
Comm::Read(serverConnection, call);
|
||
|
}
|
||
|
|
||
|
-bool
|
||
|
-HttpStateData::maybeMakeSpaceAvailable(bool doGrow)
|
||
|
+/// Desired inBuf capacity based on various capacity preferences/limits:
|
||
|
+/// * a smaller buffer may not hold enough for look-ahead header/body parsers;
|
||
|
+/// * a smaller buffer may result in inefficient tiny network reads;
|
||
|
+/// * a bigger buffer may waste memory;
|
||
|
+/// * a bigger buffer may exceed SBuf storage capabilities (SBuf::maxSize);
|
||
|
+size_t
|
||
|
+HttpStateData::calcReadBufferCapacityLimit() const
|
||
|
{
|
||
|
- // how much we are allowed to buffer
|
||
|
- const int limitBuffer = (flags.headers_parsed ? Config.readAheadGap : Config.maxReplyHeaderSize);
|
||
|
-
|
||
|
- if (limitBuffer < 0 || inBuf.length() >= (SBuf::size_type)limitBuffer) {
|
||
|
- // when buffer is at or over limit already
|
||
|
- debugs(11, 7, "will not read up to " << limitBuffer << ". buffer has (" << inBuf.length() << "/" << inBuf.spaceSize() << ") from " << serverConnection);
|
||
|
- debugs(11, DBG_DATA, "buffer has {" << inBuf << "}");
|
||
|
- // Process next response from buffer
|
||
|
- processReply();
|
||
|
- return false;
|
||
|
+ if (!flags.headers_parsed)
|
||
|
+ return Config.maxReplyHeaderSize;
|
||
|
+
|
||
|
+ // XXX: Our inBuf is not used to maintain the read-ahead gap, and using
|
||
|
+ // Config.readAheadGap like this creates huge read buffers for large
|
||
|
+ // read_ahead_gap values. TODO: Switch to using tcp_recv_bufsize as the
|
||
|
+ // primary read buffer capacity factor.
|
||
|
+ //
|
||
|
+ // TODO: Cannot reuse throwing NaturalCast() here. Consider removing
|
||
|
+ // .value() dereference in NaturalCast() or add/use NaturalCastOrMax().
|
||
|
+ const auto configurationPreferences = NaturalSum<size_t>(Config.readAheadGap).value_or(SBuf::maxSize);
|
||
|
+
|
||
|
+ // TODO: Honor TeChunkedParser look-ahead and trailer parsing requirements
|
||
|
+ // (when explicit configurationPreferences are set too low).
|
||
|
+
|
||
|
+ return std::min<size_t>(configurationPreferences, SBuf::maxSize);
|
||
|
+}
|
||
|
+
|
||
|
+/// The maximum number of virgin reply bytes we may buffer before we violate
|
||
|
+/// the currently configured response buffering limits.
|
||
|
+/// \retval std::nullopt means that no more virgin response bytes can be read
|
||
|
+/// \retval 0 means that more virgin response bytes may be read later
|
||
|
+/// \retval >0 is the number of bytes that can be read now (subject to other constraints)
|
||
|
+std::optional<size_t>
|
||
|
+HttpStateData::canBufferMoreReplyBytes() const
|
||
|
+{
|
||
|
+#if USE_ADAPTATION
|
||
|
+ // If we do not check this now, we may say the final "no" prematurely below
|
||
|
+ // because inBuf.length() will decrease as adaptation drains buffered bytes.
|
||
|
+ if (responseBodyBuffer) {
|
||
|
+ debugs(11, 3, "yes, but waiting for adaptation to drain read buffer");
|
||
|
+ return 0; // yes, we may be able to buffer more (but later)
|
||
|
+ }
|
||
|
+#endif
|
||
|
+
|
||
|
+ const auto maxCapacity = calcReadBufferCapacityLimit();
|
||
|
+ if (inBuf.length() >= maxCapacity) {
|
||
|
+ debugs(11, 3, "no, due to a full buffer: " << inBuf.length() << '/' << inBuf.spaceSize() << "; limit: " << maxCapacity);
|
||
|
+ return std::nullopt; // no, configuration prohibits buffering more
|
||
|
}
|
||
|
|
||
|
+ const auto maxReadSize = maxCapacity - inBuf.length(); // positive
|
||
|
+ debugs(11, 7, "yes, may read up to " << maxReadSize << " into " << inBuf.length() << '/' << inBuf.spaceSize());
|
||
|
+ return maxReadSize; // yes, can read up to this many bytes (subject to other constraints)
|
||
|
+}
|
||
|
+
|
||
|
+/// prepare read buffer for reading
|
||
|
+/// \return the maximum number of bytes the caller should attempt to read
|
||
|
+/// \retval 0 means that the caller should delay reading
|
||
|
+size_t
|
||
|
+HttpStateData::maybeMakeSpaceAvailable(const size_t maxReadSize)
|
||
|
+{
|
||
|
// how much we want to read
|
||
|
- const size_t read_size = calcBufferSpaceToReserve(inBuf.spaceSize(), (limitBuffer - inBuf.length()));
|
||
|
+ const size_t read_size = calcBufferSpaceToReserve(inBuf.spaceSize(), maxReadSize);
|
||
|
|
||
|
- if (!read_size) {
|
||
|
+ if (read_size < 2) {
|
||
|
debugs(11, 7, "will not read up to " << read_size << " into buffer (" << inBuf.length() << "/" << inBuf.spaceSize() << ") from " << serverConnection);
|
||
|
- return false;
|
||
|
+ return 0;
|
||
|
}
|
||
|
|
||
|
- // just report whether we could grow or not, do not actually do it
|
||
|
- if (doGrow)
|
||
|
- return (read_size >= 2);
|
||
|
-
|
||
|
// we may need to grow the buffer
|
||
|
inBuf.reserveSpace(read_size);
|
||
|
- debugs(11, 8, (!flags.do_next_read ? "will not" : "may") <<
|
||
|
- " read up to " << read_size << " bytes info buf(" << inBuf.length() << "/" << inBuf.spaceSize() <<
|
||
|
- ") from " << serverConnection);
|
||
|
-
|
||
|
- return (inBuf.spaceSize() >= 2); // only read if there is 1+ bytes of space available
|
||
|
+ debugs(11, 7, "may read up to " << read_size << " bytes info buffer (" << inBuf.length() << "/" << inBuf.spaceSize() << ") from " << serverConnection);
|
||
|
+ return read_size;
|
||
|
}
|
||
|
|
||
|
/// called after writing the very last request byte (body, last-chunk, etc)
|
||
|
diff --git a/src/http.h b/src/http.h
|
||
|
index e70cd7e..f7ed40d 100644
|
||
|
--- a/src/http.h
|
||
|
+++ b/src/http.h
|
||
|
@@ -15,6 +15,8 @@
|
||
|
#include "http/StateFlags.h"
|
||
|
#include "sbuf/SBuf.h"
|
||
|
|
||
|
+#include <optional>
|
||
|
+
|
||
|
class FwdState;
|
||
|
class HttpHeader;
|
||
|
class String;
|
||
|
@@ -112,16 +114,9 @@ private:
|
||
|
|
||
|
void abortTransaction(const char *reason) { abortAll(reason); } // abnormal termination
|
||
|
|
||
|
- /**
|
||
|
- * determine if read buffer can have space made available
|
||
|
- * for a read.
|
||
|
- *
|
||
|
- * \param grow whether to actually expand the buffer
|
||
|
- *
|
||
|
- * \return whether the buffer can be grown to provide space
|
||
|
- * regardless of whether the grow actually happened.
|
||
|
- */
|
||
|
- bool maybeMakeSpaceAvailable(bool grow);
|
||
|
+ size_t calcReadBufferCapacityLimit() const;
|
||
|
+ std::optional<size_t> canBufferMoreReplyBytes() const;
|
||
|
+ size_t maybeMakeSpaceAvailable(size_t maxReadSize);
|
||
|
|
||
|
// consuming request body
|
||
|
virtual void handleMoreRequestBodyAvailable();
|