Bug 5352: Do not get stuck when RESPMOD is slower than read(2) (#1777)
... RESPMOD BodyPipe buffer becomes full ...
maybeMakeSpaceAvailable: will not read up to 0
The AsyncCall Client::noteDelayAwareReadChance constructed
... RESPMOD consumes some buffered virgin body data ...
entering BodyProducer::noteMoreBodySpaceAvailable
leaving BodyProducer::noteMoreBodySpaceAvailable
When RESPMOD does not empty its adaptation BodyPipe buffer fast enough,
readReply() may eventually fill that buffer and call delayRead(),
anticipating a noteDelayAwareReadChance() callback from Store or Server
delay pools. That callback never happens if Store and Server are not
getting any data -- they do not even start working until RESPMOD service
starts releasing adapted/echoed response back to Squid! Meanwhile, our
flags.do_next_read (cleared by readReply() caller) remains false.
When/if RESPMOD service eventually frees some BodyPipe buffer space,
triggering noteMoreBodySpaceAvailable() notification, nothing changes
because maybeReadVirginBody() quits when flags.do_next_read is false.
noteMoreBodySpaceAvailable() could not just make flags.do_next_read true
because that flag may be false for a variety of other/permanent reasons.
Instead, we replaced that one-size-fits-all flag with more specific
checks so that reading can resume if it is safe to resume it. This
change addresses a couple of flag-related XXXs.
The bug was introduced in 2023 commit 50c5af88. Prior that that change,
delayRead() was not called when RESPMOD BodyPipe buffer became full
because maybeMakeSpaceAvailable() returned false in that case, blocking
maybeReadVirginBody() from triggering readReply() via Comm::Read(). We
missed flags.do_next_read dependency and that Store-specific delayRead()
cannot be used to wait for adaptation buffer space to become available.
XXX: To reduce risks, this change duplicates a part of
calcBufferSpaceToReserve() logic. Removing that duplication requires
significant (and risky) refactoring of several related methods.
4712608...
by
Francesco Chemolli <email address hidden>
Add AsList::quoted() (#1779)
4f3f75b...
by
Amos Jeffries <email address hidden>
Upgrade Acl::Node::name to SBuf; remove AclMatchedName global (#1766)
AclMatchedName global has been localized into a regular Acl::Answer data
member (Acl::Answer maintains the result of ACLChecklist evaluations).
This long overdue change resolves an old TODO and XXXs, paving the way
for Acl::Node reference counting.
No significant functionality changes are expected, but it is possible
that some deny_info configurations will now be handled better in
reconfiguration corner cases (because Squid no longer forgets the name
of the last checked ACL when a slow ACL check crosses reconfiguration
barrier).
Most of these changes are performance-neutral or -positive because they
eliminate or reduce memory allocations and associated name copying (and
more reduction will become possible after upgrading squid.conf parsers
to use SBuf). This change adds SBuf object copies when Acl::Answer is
propagated to ACLCB callbacks, but those read-only copies are cheap.
Also renamed and polished aclGetDenyInfoPage() because we had to update
its parameter type (to supply the last evaluated ACL). All callers were
also supplying the same first argument (that is unlikely to change in
the foreseeable future); that argument is now gone. We did not fix the
redirect_allowed name and debugs(): Fixing that behavior deserves a
dedicated change.
Also polished legacy aclIsProxyAuth() profile and description because we
have to change the parameter type (to supply the last evaluated ACL).
Also removed 63-character aclname parameter limit for acl directives.
e3aef57...
by
Francesco Chemolli <email address hidden>
NoNewGlobals for HttpHdrCc:ccLookupTable (#1750)
Detected by Coverity. CID 1554655: Initialization or destruction
ordering is unspecified (GLOBAL_INIT_ORDER).
Also switched to compile-time checks for table initialization records.
3996c0d...
by
Francesco Chemolli <email address hidden>
Add AtMostOnce stream manipulator (#1742)
1a66c4f...
by
Francesco Chemolli <email address hidden>
Refactor and improve ErrorState::Dump (#1730)
Rework the internals for generating output in ErrorState::Dump,
used for expanding the '%W' token in error page templates.
Also fix a bug with excessive html-quoting of the output.
