Skip to content

cw-pause: fix O(N^2) list traversal in cw_pause_flush - #22968

Closed
MegaManSec wants to merge 1 commit into
curl:masterfrom
MegaManSec:fix-cw-pause-on2
Closed

MegaManSec wants to merge 1 commit into
curl:masterfrom
MegaManSec:fix-cw-pause-on2

Conversation

@MegaManSec

Copy link
Copy Markdown
Contributor

Switch the pause buffer list from prepend-at-head to append-at-tail and add a tail pointer to cw_pause_ctx. cw_pause_flush can now consume nodes from the head in O(1) per node instead of re-scanning from the head on every outer iteration.

The same fix was applied to cw-out.c in 89ca8fe but cw-pause.c was missed. With many buffered nodes (e.g. from an adversarial HTTP/2 server interleaving HEADERS/DATA frames while a CURL_WRITEFUNC_PAUSE callback is active), the old code performed O(N²) work on unpause.

Switch the pause buffer list from prepend-at-head to append-at-tail and
add a tail pointer to cw_pause_ctx. cw_pause_flush can now consume nodes
from the head in O(1) per node instead of re-scanning from the head on
every outer iteration.
@testclutch

Copy link
Copy Markdown

Analysis of PR #22968 at 86e1f075:

Test ../../tests/http/test_21_resolve.py::TestResolve::test_21_08_dnsd_aaaa[DoH] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Test ../../tests/http/test_21_resolve.py::TestResolve::test_21_09_dnsd_a_delay[DoH] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Test ../../tests/http/test_21_resolve.py::TestResolve::test_21_10_dnsd_aaaa_delay[DoH] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Test ../../tests/http/test_21_resolve.py::TestResolve::test_21_13_dnsd_nxdomain_cached[DoH] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Generated by Testclutch

@bagder
bagder requested review from icing and a balanced review from Copilot September 17, 2026 08:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The queue invariants and write ordering are preserved while eliminating repeated list traversal.

Pull request overview

Optimizes paused client-write buffering by replacing quadratic list traversal with FIFO head/tail operations.

Changes:

  • Adds and maintains a tail pointer.
  • Appends buffered writes at the tail and flushes from the head in O(1) per node.
File summaries
File Description
lib/cw-pause.c Implements efficient FIFO buffering and flushing.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@bagder bagder closed this in 55ecaa5 Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants