Skip to content

LibWeb: Release discarded navigation redirect requests#10724

Merged
awesomekling merged 1 commit into
LadybirdBrowser:masterfrom
gmta:libweb-fetch-redirect-leak
Jul 17, 2026
Merged

LibWeb: Release discarded navigation redirect requests#10724
awesomekling merged 1 commit into
LadybirdBrowser:masterfrom
gmta:libweb-fetch-redirect-leak

Conversation

@gmta

@gmta gmta commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Since f454515, document navigation requests have been retained in RequestServer so they can be handed to the UI process if a response becomes a download. Intermediate redirect responses never reach download handling, however, and replacing FetchController's pending request left each retained response pipe open. Repeated redirects eventually exhausted RequestServer's file descriptors.

The Fetch specification does not account for this lifecycle. RequestServer transfer retention is a Ladybird implementation detail, not web-observable Fetch state. Release the transfer hold before discarding an intermediate redirect response, and stop its request when it is still active.

Cover this with a focused http_redirect_fetch() test that records the release of a synthetic intermediate response. An end-to-end regression would have to lower the process file limit and perform hundreds of navigations, making it slow and dependent on test-runner arguments and process timing. The unit test directly protects the missing ownership step.

This is one of the potential sources for FD leaks that we're seeing on our WPT runner.

Since f454515, document navigation requests have been retained in
RequestServer so they can be handed to the UI process if a response
becomes a download. Intermediate redirect responses never reach download
handling, however, and replacing FetchController's pending request left
each retained response pipe open. Repeated redirects eventually
exhausted RequestServer's file descriptors.

The Fetch specification does not account for this lifecycle.
RequestServer transfer retention is a Ladybird implementation detail,
not web-observable Fetch state. Release the transfer hold before
discarding an intermediate redirect response, and stop its request when
it is still active.

Cover this with a focused http_redirect_fetch() test that records the
release of a synthetic intermediate response. An end-to-end regression
would have to lower the process file limit and perform hundreds of
navigations, making it slow and dependent on test-runner arguments and
process timing. The unit test directly protects the missing ownership
step.
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

HTTP redirect fetching now releases intermediate response transfer state and stops associated requests when no location URL is produced. The fetch function is exported, and a LibWeb test verifies transfer release during redirect processing.

Changes

Redirect cleanup

Layer / File(s) Summary
Redirect cleanup and API export
Libraries/LibWeb/Fetch/Fetching/Fetching.h, Libraries/LibWeb/Fetch/Fetching/Fetching.cpp
http_redirect_fetch is exported and releases the intermediate response transfer hold, stopping an associated RequestServer request before returning when no location URL is available.
Transfer-release test coverage
Tests/LibWeb/CMakeLists.txt, Tests/LibWeb/TestFetchResponse.cpp
The test target links LibRequests and LibURL; a tracking response and redirect test verify that transfer release occurs.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: atkinssj

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description matches the patch, describing redirect request cleanup, transfer release, and the new focused http_redirect_fetch test.

Comment @coderabbitai help to get the list of available commands.

@gmta
gmta requested a review from awesomekling July 16, 2026 22:35
@awesomekling
awesomekling merged commit 48ce388 into LadybirdBrowser:master Jul 17, 2026
20 of 21 checks passed
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