Skip to content

test: Fix a flaky test_proxy_07_cache_control.py (seen on Windows CI). - #864

Closed
arturobernalg wants to merge 1 commit into
apache:trunkfrom
arturobernalg:fix/httpd-test-07-cache-control-flake
Closed

arturobernalg wants to merge 1 commit into
apache:trunkfrom
arturobernalg:fix/httpd-test-07-cache-control-flake

Conversation

@arturobernalg

@arturobernalg arturobernalg commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

The cache entry for the first request is committed by the CACHE_SAVE filter on the EOS-only pass that follows the body, which can still be running server side for an instant after the client has already read the complete body. Two back-to-back requests with no delay can race this, making the second one miss the cache and hit the backend again.

Give the commit room to land before relying on it, instead of asserting immediately after the first request returns. Reproduced locally (about 1 in 5 runs failing, on different parametrized cases each time, none of them involving a comma) and confirmed fixed (0 failures in 55 runs) before and after this change.

Also harden the TCPFaker backend used by this test to close its socket with an orderly shutdown() instead of an abrupt close(), which removes one more way for a slow platform to see a reset instead of a clean end of response.


Fixes the flaky test_proxy_07_002/003 cases in test_07_cache_control.py (added in #821), seen failing on Windows CI. Reproduced locally too, about 1 in 5 runs, on different Cache-Control cases each time, including ones with no comma at all, so it's unrelated to the comma-parsing fix in #821. Root cause: the second request can race the first one's CACHE_SAVE commit, which can still be running for a moment after the client already read the full response. Confirmed fixed, 0 failures in 55 local runs.

  • This PR does not report or address a security vulnerability
    (see SECURITY.md and
    https://www.apache.org/security/#reporting-a-vulnerability).
  • Code follows the httpd style guide.
  • New log messages (level debug or higher) use an empty APLOGNO() tag;
    numbers are assigned by a committer at merge time (see docs/log-message-tags/README).
  • If the change is user-visible, a changes-entries/*.txt file is included,
    following the template in README.CHANGES (not needed otherwise).
  • Test cases based on pyhttpd are included where appropriate, in the
    test/modules/xxx directory matching the modules/xxx module source,
    or test/modules/core for core server changes. On Unix, verify these
    pass locally with e.g. make check-pytest PYTEST_DIRS=test/modules/xxx.

The cache entry for the first request is committed by the CACHE_SAVE
filter on the EOS-only pass that follows the body, which can still be
running server side for an instant after the client has already read
the complete body. Two back-to-back requests with no delay can race
this, making the second one miss the cache and hit the backend again.

Give the commit room to land before relying on it, instead of
asserting immediately after the first request returns. Reproduced
locally (about 1 in 5 runs failing, on different parametrized cases
each time, none of them involving a comma) and confirmed fixed (0
failures in 55 runs) before and after this change.

Also harden the TCPFaker backend used by this test to close its
socket with an orderly shutdown() instead of an abrupt close(), which
removes one more way for a slow platform to see a reset instead of a
clean end of response.

@notroj notroj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants