]> git.ipfire.org Git - thirdparty/squid.git/commit
Adjusted ConnOpener API to fix several problems
authorAlex Rousskov <rousskov@measurement-factory.com>
Sun, 1 Aug 2021 21:20:48 +0000 (17:20 -0400)
committerAlex Rousskov <rousskov@measurement-factory.com>
Sun, 1 Aug 2021 21:57:40 +0000 (17:57 -0400)
commit21f1082fe26158fe613de12d0548e9d4bb7c9ace
tree9505adeebf4c1f0670d94a16c6e27f64bbc2f7c3
parentc91557fe1ef5e88093d950aedb9645f5330dbf29
Adjusted ConnOpener API to fix several problems

1. Many ConnOpener users are witten to keep a ConnectionPointer to the
   destination given to ConnOpener. This means that their connection
   magically opens when ConnOpener successfully connects, before
   ConnOpener has a chance to notify the user about the changes. Having
   multiple concurrent connection owners is always dangerous, and the
   user cannot even have a close handler registered for its now-open
   connection. When something happens to ConnOpener or its answer, the
   user job may be in trouble.

   ConnOpener now clones the destination parameter, refusing to tie
   ConnOpener connection to the ConnOpener creator connection. This
   addresses the concern, but requires adjustment 2.

2. Refactored ConnOpener users to stop assuming that the answer contains
   a pointer to their connection object. After adjustment 1 above, it
   does not. HappyConnOpener relied on that assumption quite a bit so we
   had to refactor to use two custom callback methods instead of one
   with a complicated if-statement distinguishing prime from spare
   attempts. This refactoring is an overall improvement because it
   simplifies the code. Other ConnOpener users just needed to remove a
   few no longer valid paranoid assertions/Musts.

3. ConnOpener users were forced to remember to close params.conn when
   processing negative answers. Some, naturally, forgot, triggering
   warnings about orphaned connections (e.g., Ident and TcpLogger).
   ConnOpener now closes its open connection before sending a negative
   answer.

4. ConnOpener would trigger orphan connection warnings if the job ended
   after opening the connection but without supplying the connection to
   the requestor (e.g., because the requestor has gone away). Now,
   ConnOpener explicitly closes its open connection if it has not been
   sent to the requestor.

Also fixed Comm::ConnOpener::cleanFd() debugging that was incorrectly
saying that the method closes the temporary descriptor.

Also added Comm::Connection::cloneDestinationDetails() debugging to
simplify tracking dependencies between half-baked Connection objects
carrying destination/flags/other metadata and open Connection objects
created by ConnOpener using that metadata (which are later delivered to
ConnOpener users and, in some cases, replace those half-baked
connections mentioned earlier. Long-term, we need to find a better way
to express these and other Connection states/stages than comments and
debugging messages.
src/HappyConnOpener.cc
src/HappyConnOpener.h
src/PeerPoolMgr.cc
src/adaptation/icap/Xaction.cc
src/comm/ConnOpener.cc
src/comm/ConnOpener.h
src/comm/Connection.cc
src/ident/Ident.cc
src/servers/FtpServer.cc