Skip to content

WIP: progressive read timeout review - #1

Open
BGQ99 wants to merge 4 commits into
masterfrom
review/pr3409-fixes
Open

WIP: progressive read timeout review#1
BGQ99 wants to merge 4 commits into
masterfrom
review/pr3409-fixes

Conversation

@BGQ99

@BGQ99 BGQ99 commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Summary

This draft PR prepares the progressive read timeout changes from apache#3409 for review.

Changes

  • Rework progressive read timeout tracking around an idle deadline.
  • Refresh the deadline after each successful OnReadOnePart() callback.
  • Keep timer callbacks independent from the wrapper and user reader lifetime.
  • Close the HTTP/1.x socket through Socket::SetFailed() instead of destroying the parsing context from the timer thread.
  • Ensure the final reader callback is delivered at most once.
  • Preserve errors returned by the user progressive reader.
  • Reject progressive read timeout on HTTP/2 with ENOTSUP.
  • Restore SocketId plumbing and initialize _socket_id with INVALID_SOCKET_ID.
  • Register EPROGREADTIMEOUT.
  • Add timeout, slow-callback, reader-error, HTTP/2, and callback-lifecycle tests.
  • Fix the http_c++ example buffer handling, reader cleanup, and spelling.

Verification

  • Build completed successfully.
  • Progressive timeout tests passed repeatedly.
  • HTTP test suite: 52/52 passed.
  • git diff --check passed.

Known Test Coverage Gap

The active progressive-read test currently verifies successful continuous reading, but does not yet explicitly guarantee that the total transfer time exceeds the configured timeout while each inter-read interval remains below the timeout.

The production deadline-refresh logic is covered by the current behavior tests. The active-read timing test should be strengthened in a follow-up revision.

This is a draft for colleague review. No merge is intended yet.

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