[squid-users] Adaptation error

[Alex Rousskov]

On 6/4/20 7:56 PM, senor wrote:

> The only solution we had come up with was to eliminate the assert
> because it didn't seem to be of concern. Unless you think eliminating
> the assert is not a good idea, I think my question is answered.

AFAICT, the code should continue to work despite the violation of the
asserted condition. Needless to say, that conclusion may change in
future Squid versions, and my superficial analysis itself can be wrong.

I recommend filing a bug report in Squid bugzilla with a stack trace and
a reference to this email thread.


Thank you,

Alex.


> From: Alex Rousskov <rousskov at measurement-factory.com>
> Sent: Thursday, June 4, 2020 2:55 PM
> To: senor; squid-users at lists.squid-cache.org
> Subject: Re: [squid-users] Adaptation error
> 
> On 6/4/20 4:15 PM, senor wrote:
> 
>> I occasionally get an assert and core dump on a worker here:
> 
>> #3  in ClientHttpRequest::resetRequest () at client_side_request.cc:1676
> 
> I assume you are hitting assert(request != newRequest).
> 
> 
>> #4  in ClientHttpRequest::handleAdaptedHeader () at client_side_request.cc:2005
> 
> 
>> As I understand it, the request returned from adaptation is compared
>> with pre-adapted request and it is considered a fail if they match.
>> Since there is no requirement that adaptation make a change, I assume
>> it should not be in that code unless adaptation indicated there was a
>> change.
> 
> IIRC, Squid ICAP client code creates a new request structure even if no
> adaptation happened at the ICAP service. This is a deficiency of that
> old code (some may say a bug, but there could have been good reasons to
> make it that way back then because that request structure is "special"
> and may have been difficult to reuse -- I am not sure).
> 
> Apparently, there is some other adaptation code that does not create a
> new request structure. I speculate that it may be the request routing
> code that lives outside the ICAP space (it also serves eCAP adapters).
> For a specific example, see Adaptation::Iterator::step() -- I see no
> obvious/immediate signs there that theMsg is a new message and not the
> virgin one. IIRC, theMsg starts as a virgin message structure.
> 
> I suspect that the correct fix would be to stop requiring a new request
> structure for the akForward Answers. Instead, handleAdaptedHeader()
> should accommodate the case where the request structure was preserved
> and does not need to be reset. In other words, a Forward(x) answer
> should not imply that the original message was (not) adapted.
> 
> Other Answer recipients should be checked for a similar problem.
> 
> 
> HTH,
> 
> Alex.
> _______________________________________________
> squid-users mailing list
> squid-users at lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-users
>