Support LeakSanitizer#3361
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves compatibility with LeakSanitizer (LSan) by reducing real leaks and suppressing/avoiding expected “lifetime” allocations that would otherwise be reported at process exit, across both unit tests and long-lived runtime threads/singletons.
Changes:
- Fix/suppress numerous test leaks by adding missing cleanup, switching to RAII in tests, and annotating intentionally-leaky allocations.
- Adjust runtime components to avoid LSan false-positives (e.g., leak annotations in long-lived loops/threads; unpoison pooled objects before leak check).
- Minor lifecycle/concurrency cleanup (e.g., TimerThread stop idempotence; bthread TLS/span teardown ordering).
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/thread_key_unittest.cpp | Switches TLS test allocations toward RAII to avoid leak reports (but requires care with TLS destructors). |
| test/run_tests.sh | Enables leak detection in ASan options for test runs by default. |
| test/object_pool_unittest.cpp | Prevents intentional per-iteration new int leaks by retaining allocations for test lifetime. |
| test/iobuf_unittest.cpp | Returns acquired TLS blocks to avoid leaks in the unit test. |
| test/CMakeLists.txt | Avoids linking gperftools when ASan is enabled (ASan incompatibility). |
| test/BUILD.bazel | Marks an additional test as large to reduce flakiness/resource contention. |
| test/bthread_key_unittest.cpp | Uses RAII to ensure thread-allocated args are freed. |
| test/bthread_fd_unittest.cpp | Replaces raw owning pointers with std::unique_ptr for test metadata. |
| test/bthread_dispatcher_unittest.cpp | Adds finite poll timeouts and explicit cleanup of allocated resources/fds. |
| test/brpc_ssl_unittest.cpp | Frees OpenSSL objects (X509/SSL/SSL_CTX) to avoid test leaks. |
| test/brpc_socket_unittest.cpp | Adds leak annotations for intentionally leaked objects; fixes lifetime/self-destruction and buffer cleanup paths. |
| test/brpc_redis_unittest.cpp | Uses RAII for authenticator and command handlers to prevent test leaks. |
| test/brpc_redis_cluster_unittest.cpp | Makes command handlers owned by the test service instance (RAII) to avoid leaks. |
| test/brpc_protobuf_json_unittest.cpp | Replaces heap buffer with stack buffer to avoid a leak. |
| test/brpc_proto_unittest.cpp | Uses std::unique_ptr for dynamically created protobuf messages. |
| test/brpc_load_balancer_unittest.cpp | Avoids leaking user objects on non-created sockets; uses RAII for load balancer. |
| test/brpc_input_messenger_unittest.cpp | Frees per-thread buffers and deletes allocated client metadata. |
| test/brpc_http_rpc_protocol_unittest.cpp | Explicitly releases stream user data to avoid leaking request objects. |
| test/brpc_event_dispatcher_unittest.cpp | Adds destructor cleanup and explicit socket/metadata teardown to prevent leaks. |
| test/brpc_alpn_protocol_unittest.cpp | Frees SSL/SSL_CTX objects in ALPN helper to avoid leaks. |
| src/butil/object_pool_inl.h | Registers an atexit hook to unpoison pooled objects so LSan can correctly see reachability. |
| src/butil/memory/singleton.h | Removes LSan leak-annotation behavior from Singleton creation paths. |
| src/butil/logging.cc | Annotates async logging enqueue path to suppress expected long-lived allocations under LSan. |
| src/butil/lazy_instance.h | Removes an unused include, keeping leak annotations available. |
| src/butil/find_cstr.h | Annotates thread-local string cache allocations as intentional non-leaks. |
| src/butil/debug/leak_annotations.h | Refines LSan API surface and adds explicit disable/enable macros. |
| src/bthread/timer_thread.cpp | Makes stop_and_join() idempotent via atomic exchange. |
| src/bthread/task_group.cpp | Reorders span cleanup before keytable cleanup to prevent re-populating TLS and leaking. |
| src/brpc/span.cpp | Adjusts span submission allocation behavior (now unconditional allocation). |
| src/brpc/socket_map.cpp | Adds scoped leak annotations in long-lived watch loop to avoid LSan noise. |
| src/brpc/server.cpp | Removes nothrow on builtin service allocations; fixes builtin-service ownership on Add failure; scopes leak annotation for intentional leaks. |
| src/brpc/rdma/block_pool.cpp | Deletes per-bucket mutexes and other globals in pool teardown to prevent leaks. |
| src/brpc/global.cpp | Uses explicit LSan disable/enable around long-lived object construction and scoped suppression around periodic enumeration. |
| src/brpc/controller.cpp | Clears _span earlier during controller reset to reduce retained references. |
| .github/workflows/ci-linux.yml | Enables with_bthread_tracer define in Bazel CI invocation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
LGTM |
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
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.
What problem does this PR solve?
Issue Number: resolve
Problem Summary:
What is changed and the side effects?
Changed:
Side effects:
Performance effects:
Breaking backward compatibility:
Check List: