A 304 during --update leaks the response's header buffer#808
Merged
Conversation
back_wait() handles a 304 by replacing the response struct with the cache entry, carrying only the socket and keep-alive members across via back_connxfr(). The struct assignment drops every owned pointer the live response still held without freeing any of them: the 8 KB header buffer on every update, plus the two WARC header stashes when --warc-file is on. An update over a 10k-page site drops roughly 80 MB in one run. back_clear_entry() already knew how to tear those down, so the frees move into a helper that both it and the 304 path call. The new test runs the two-pass mini304 crawl with LeakSanitizer on, which the sanitized CI job otherwise disables. The fresh first pass is the control: it has no cache entry to read back and is clean either way. The update pass reports 16 KB in 2 objects on master, one per unchanged URL, and nothing with the fix. Closes #782 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
back_free_response() was reimplementing deleteaddr(), which already frees adr and headers and NULLs both; call it instead so the two cannot drift. Test 114 never passed --warc-file, so the warc_free_request() limb ran with both pointers NULL on every path it exercised and deleting it kept the test green. A third pass turns the archive on, and it now fails with the 835 and 238 byte stashes when that call goes away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
xroche
changed the title
The Win32/x64 job pins the exact set of tests allowed to skip, so an all-skipped suite cannot report green. 114_local-update-304-leak needs a LeakSanitizer build and MSVC has no equivalent, so it skips there and tripped the gate with fail=0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
Resolve the tests/Makefile.am TESTS-tail conflict: keep both 111 and 114 entries in numeric order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
Resolve the tests/Makefile.am TESTS-tail conflict: keep 110, 111 and 114 in numeric order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
back_wait()handles a 304 by replacing the response struct with the cache entry, carrying only the socket and keep-alive members across viaback_connxfr(). The struct assignment drops the pointers the live response still owned, freeing none of them. Measured on a two-URL mini304 update: 16 KB in 2 objects, the 8 KB header blockhttp_xfread1()allocated, one per unchanged URL. By that pointr.adrhas been moved intor.headers, soheadersis what actually leaks. With--warc-filethe two stashed WARC header blocks go with it, another 1 KB per pass.back_clear_entry()already knew how to tear those down, so the frees move into a helper that both it and the 304 path call, built on the existingdeleteaddr().The test runs the mini304 crawl with LeakSanitizer on, which the sanitized CI job otherwise disables. The fresh first pass is the control: it has no cache entry to read back and is clean either way. Removing either half of the helper puts it back red, the header block on the update pass and the WARC stashes on the archive pass.
Closes #782