[this should be on squid-dev instead] On 02/09/2016 01:20 PM, Prashanth Prabhu wrote: > Here's the behavior I have seen: When the connection is set up, the > buffer gets a size of 16KB (default). Squid reads from the socket, > parses the data, and then sends it towards c-icap as appropriate. Now, > as part of parsing the data, the buffer is NUL-terminated via a call > to c_str(). This NUL-termination, however, is not accounted for by an > increase in the "offset" (off) in the underlying MemBlob, therefore, > the offset and size go out of sync. Just to avoid a misunderstanding: * MemBlob does not have an "offset". * SBuf::off_ should not change when we are adding characters to SBuf because it is the start of the buffer, not the end of it. * A call to c_str() should not increase SBuf::len_ either because it does not add a new character to the SBuf object. That call just terminates the underlying buffer. Based on your comments below, I think I know what you mean by "go out of sync", but everything is as "in sync" as it can be when one adds termination characters that are not really there from SBuf::length() point of view. The bug is elsewhere. > MemBlob::canAppend() failing because > MemBlob::isAppendOffset() fails -- the 'off' and 'size' are not the > same due to the above c_str() call. Single-owner optimizations aside (a known TODO), the above is the desired behavior according to the documented c_str() guarantees: > * The returned value points to an internal location whose contents > * are guaranteed to remain unchanged only until the next call > * to a non-constant member function of the SBuf object. In other words, we cannot allow some _other_ SBuf object to overwrite our null-termination character in the MemBlob we share with that other SBuf. The high price for that strong guarantee is one of the reasons we should avoid c_str() calls in Squid code. > When canAppend() fails, a new > buffer is re-allocated. When this reallocation occurs, however, the > new size of the buffer is dependent on the size being reserved. If we are still talking about the I/O buffer (and not just some random SBuf string somewhere), then the I/O buffer _capacity_ should not shrink below a certain minimum, regardless of how much content the buffer has already stored. There should be some Squid code that ensures the minimum capacity of the I/O buffer used to read requests. If it is missing, it is a Squid bug. > As a temporary measure, I have an experimental change that checks > whether the body size is known and if known always reserves a large > enough size (currently 16K). It is difficult to discuss this without seeing your changes, but the reservation should probably be unconditional -- the I/O buffer capacity should always be at least 16KB (or whatever size we start with). HTH, Alex. _______________________________________________ squid-users mailing list squid-users@xxxxxxxxxxxxxxxxxxxxx http://lists.squid-cache.org/listinfo/squid-users
