Skip to content

Clean up stale LSAN suppression entries#13029

Merged
bryancall merged 6 commits intoapache:masterfrom
bryancall:fix/cleanup-lsan-suppressions
Apr 13, 2026
Merged

Clean up stale LSAN suppression entries#13029
bryancall merged 6 commits intoapache:masterfrom
bryancall:fix/cleanup-lsan-suppressions

Conversation

@bryancall
Copy link
Copy Markdown
Contributor

@bryancall bryancall commented Mar 27, 2026

Problem

The LSAN suppression files contained entries for tests and libraries that no longer exist (deleted regression tests, OpenSSL 1.0.2, PCRE1), and test_Hdrs had real HTTPHdr memory leaks on early-return error paths.

Changes

  • Fix HTTPHdr leaks in test helpers -- use ts::PostScript scope guards in test_http_hdr_print_and_copy_aux and test_http_hdr_copy_over_aux instead of manual destroy() on every error path. HdrHeapSDKHandle::destroy() is idempotent so the mid-function destroy/re-create cycle remains safe.
  • Remove goto/done pattern -- replace goto done with if-not-error chaining in test_http_hdr_copy_over_aux now that PostScript handles cleanup.
  • Remove 3 stale regression suppressions -- TSHttpConnectIntercept, TSHttpConnectServerIntercept (removed), make_log_host (Lua removed), Cache_vol and Hdrs (converted to Catch2).
  • Remove 6 obsolete unit test suppressions -- OpenSSL 1.1 libcrypto.so.1.1, OpenSSL 1.0.2 CRYPTO_malloc/CRYPTO_realloc, ConsCell (symbol gone), pcre_jit_stack_alloc (PCRE replaced with PCRE2).
  • Restore TSHttpConnect suppressions -- generate_request still leaks; keep these entries with updated comments.

Testing

  • Built with ENABLE_ASAN=ON and ran unit tests -- test_Hdrs passes clean
  • CI (all 15 checks green)

Add missing HTTPHdr::destroy() calls for hdr and new_hdr on
early return paths. marshal_hdr is intentionally not destroyed
because it holds a reference to a stack-allocated TestRefCountObj.
Remove 5 entries for code that no longer exists:
- RegressionTest_SDK_API_TSHttpConnectIntercept (removed)
- RegressionTest_SDK_API_TSHttpConnectServerIntercept (removed)
- make_log_host (removed with Lua support)
- RegressionTest_Cache_vol (converted to Catch2)
- RegressionTest_Hdrs (converted to Catch2)
Remove 5 entries for obsolete libraries:
- libcrypto.so.1.1: OpenSSL 1.1 specific, test cleanup fixed
- CRYPTO_malloc/CRYPTO_realloc: OpenSSL 1.0.2 (minimum is 1.1.1)
- ConsCell: symbol no longer exists in codebase
- pcre_jit_stack_alloc: PCRE replaced with PCRE2

Keep test_http_hdr_print_and_copy_aux suppression with updated
comment explaining why marshal_hdr is intentionally not destroyed.
@bryancall bryancall added this to the 11.0.0 milestone Mar 27, 2026
@bryancall bryancall added Tests Bug ASan Address Sanitizer labels Mar 27, 2026
@bryancall bryancall self-assigned this Mar 27, 2026
bryancall

This comment was marked as off-topic.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Cleans up leak-sanitizer suppression lists by removing entries for deleted/converted tests and tightening unit-test leak behavior by fixing early-return cleanup in test_Hdrs.

Changes:

  • Fix early-return HTTPHdr cleanup in test_http_hdr_copy_over_aux and test_http_hdr_print_and_copy_aux to eliminate real leaks.
  • Remove stale/obsolete leak suppression entries from ci/asan_leak_suppression/regression.txt and ci/asan_leak_suppression/unit_tests.txt.
  • Document the remaining unit-test suppression for test_http_hdr_print_and_copy_aux.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/proxy/hdrs/unit_tests/test_Hdrs.cc Adds missing destroy() calls on error paths to prevent HTTPHdr heap leaks.
ci/asan_leak_suppression/unit_tests.txt Drops obsolete suppressions and keeps a single, documented suppression for test_http_hdr_print_and_copy_aux.
ci/asan_leak_suppression/regression.txt Removes suppression entries for regression tests/symbols no longer present in the tree.

Comment thread ci/asan_leak_suppression/unit_tests.txt
Comment thread src/proxy/hdrs/unit_tests/test_Hdrs.cc Outdated
Comment thread src/proxy/hdrs/unit_tests/test_Hdrs.cc Outdated
masaori335
masaori335 previously approved these changes Mar 30, 2026
Replace manual destroy() calls on every error path with
ts::PostScript scope guards. HdrHeapSDKHandle::destroy() is
idempotent (checks m_heap before acting), so the mid-function
destroy/re-create cycle in test_http_hdr_print_and_copy_aux
is safe alongside the guard.
Now that PostScript handles cleanup, replace goto with
if-not-error chaining for the comparison steps.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown
Contributor

@masaori335 masaori335 left a comment

Choose a reason for hiding this comment

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

Thank you, it's much cleaner.

@bryancall bryancall merged commit 77beafa into apache:master Apr 13, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ASan Address Sanitizer Bug Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants