[squid-dev] File descriptor leak at ICAP reqmod rewrites of CONNECT requests

[Alex Rousskov]

On 12/10/20 3:33 PM, Alexey Sergin wrote:

> - Squid writes to cache.log a message like "kick abandoning <....>";

These messages indicate a Squid bug, most likely in REQMOD request
satisfaction implementation specific to CONNECT use cases. The messages
are not prefixed with a "BUG" label, but they should be.


> - Squid does not close the file descriptor used for http client connection.

Yes, that is a likely side effect of the above-mentioned bug.


> client_side.cc. Closing clientConnection right after
> "debugs(<....>abandoning<....>)" fixes the leak.

> Is it ok to always close() clientConnection when "abandoning" thing
> happens? 

> Are there any known scenarios where this close() would be
> inappropriate?

Unknown. Such a closure (alone) is not a proper fix. If well-tested, it
may be considered to be a good-enough workaround, but no more than that.

What is currently missing (at least) is understanding of what is going
on. The correct fix, whatever it is, would be determined by that
understanding.

In general, responding with a 403 does not invalidate the client
connection, so it does not have to be closed. Said that, I am sure there
are use cases where such closure would be a good idea. I would not be
surprised if Squid closes the connection after denying regular requests.


> Could you please give me some advice on a better/proper fix, if close()
> at "abandoning" time is wrong?

Unfortunately, I cannot. Somebody needs to investigate the problem and
identify the bug in ConnStateData::kick() logic (or elsewhere).

Alex.