Repository navigation
Resolve modular CMake conflict and preserve handshake protobuf generation - #82
Merged
zchuango merged 5 commits intoOct 20, 2026
Conversation
…pache#3590) * Destroy keytable pool and tls key when Server::Start() fails early StartInternal() allocates _keytable_pool (and the tls key when a thread_local_data_factory is set) unconditionally, but on failures before _status reaches RUNNING - e.g. tcp_listen() fails because the port is in use - RevertServerStatus runs Stop(0)+Join(), both of which return immediately when the server is not RUNNING. The pool and the key leaked on every failed attempt, and a retry overwrote _keytable_pool with a fresh pool, permanently leaking the previous one. ~Server() had the same problem for a failed-start server. Add Server::DestroyPreStartAllocations() (no-op once RUNNING/STOPPING, where Join() handles cleanup) and call it from RevertServerStatus and ~Server(). Unlike Join() the pool struct is deleted too, since no connection can exist before RUNNING. Verified in ServerTest.sanity: after a port-conflict failure, _keytable_pool is nullptr and the tls key is invalid (on the old code both leak and the test fails), including across a failed retry. Full brpc_server_unittest suite shows no new failures. * Bthreads spawned from bthread_init_fn with the inherited attr (bthread_getattr(bthread_self(), ...)) carry keytable_pool == _keytable_pool and may outlive a failed Server::Start(). They still call return_keytable() on exit, so deleting the pool struct in DestroyPreStartAllocations() causes a heap-use-after-free (reported by ASan at key.cpp:394). Do the same as Join(): destroy the pool contents with bthread_keytable_pool_destroy() but don't delete the struct. A destroyed pool makes late return_keytable() calls delete their KeyTable directly. The leftover struct is small, only leaked on failed starts, and already annotated in StartInternal(). Add failed_start_keeps_keytable_pool_for_init_bthreads, which spawns such a child from bthread_init_fn and fails Start() with EADDRINUSE. * test(brpc): Synchronize failed-start regression test Replace the fixed sleep with a CountdownEvent so the child bthread continues only after the failed Start() returns and the Server is destroyed. This ensures that the regression test exercises access to the destroyed keytable pool. Use a nonfatal Start() assertion so the child is still signaled and joined if startup unexpectedly succeeds. * refactor(server): share thread-local cleanup Centralize keytable pool and TLS key cleanup for normal shutdown and failed-start recovery while preserving their destruction order and pool lifetime constraints.
…m init (apache#3627) * fix enqueue move-only * Add test for move-only enqueue and dequeue Added test for enqueueing and dequeuing move-only types in MPSCQueue. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…pache#3618) * Move CMake source definitions into module directories Keep source selection and bundled artifacts unchanged while moving each module source list and RPC protocol generation out of the root project. Retain the existing test source inputs during this first migration step. * Define CMake production targets in their module directories Let each module own its production object library and move the mcpack protoc generator into its module. Aggregate the module objects into the existing brpc bundle without compiling production sources twice. Move butil configuration-header generation, feature-option handling and dependency discovery into the owning modules. Keep RPC-only headers out of lower-library compile settings and assemble legacy bundle dependencies after module setup. Apply CRC32C instruction flags only to the relevant sources, including debug builds. Preserve the RISC-V vector base64 options and the existing AArch64 libcontext workaround. Query module targets for the existing unit-test source sets instead of passing source lists through parent directory scopes. Preserve the test compile definitions and bundled debug library linkage. Install source and generated headers directly from their owning modules. Keep build-tree include paths separate from installed target interfaces instead of copying headers into an output staging tree. Use Protobuf's imported targets to preserve static transitive dependencies and Debug/Release library selection, and handle config-mode import paths. * Use explicit installation prefixes for CMake examples Stop searching for the removed output/include staging directory and honor the caller-provided CMAKE_PREFIX_PATH in both example entry points. Document the install-before-use workflow in English and Chinese. Link Protobuf through its imported target so non-system installations supply the headers required by generated sources and public RPC headers. * Align the ubring example CMake minimum with the project Require CMake 3.16 and use the same policy range as the root project and other examples. This makes the declared minimum consistent with use of the protobuf::libprotobuf imported target introduced in CMake 3.9. * Configure the install prefix before generating pkg-config metadata Set CMAKE_INSTALL_PREFIX during configuration in both getting-started guides, then build and install without an install-time prefix override. This keeps the generated brpc.pc paths aligned with the installed files. * Prefer generated IDL headers when building the mcpack generator Inherit build settings through the selected brpc bundle instead of linking butil_build_config first. This keeps the generated IDL header directory ahead of the source tree and prevents stale Make-generated headers from shadowing the current CMake output. * Remove unused CMake module configuration leftovers Drop the unread platform-library property and unused URMA parent-scope exports. Remove redundant debug-bundle glog blocks, since module-owned bundle dependencies already propagate the library through DYNAMIC_LIB. * Document installed-prefix builds for UBRing examples Set the brpc installation prefix at configure time, install the bundle, and pass the prefix explicitly when building the performance example. Update both build sections in the English and Chinese UBRing guides. * Preserve ordered transitive dependencies for CMake bundles Propagate the complete link dependency closure from the static bundle and reuse it in tools rather than masking missing dependencies at each caller. Order module consumers before providers in both CMake and pkg-config, and keep system libraries after their users. Retain LevelDB Snappy dependencies independently of WITH_SNAPPY without exposing unused Snappy headers. Include selected optional runtime libraries in the generated private link metadata. Add bundle-only consumers and dependency-metadata checks to the existing CMake CI jobs without increasing job concurrency. * Normalize legacy LevelDB CMake target dependencies Use LevelDB's exported target to carry its link dependencies while keeping header and library discovery for installations without a CMake package. Work around private warning flags and unnecessary shared-library Snappy linkage exported by LevelDB 1.23. Preserve static and unknown-type link dependencies, along with brpc's explicit WITH_SNAPPY option. Give debug sources the same LevelDB requirements and add a standalone consumer to the existing CMake link checks without adding CI jobs.
…ves it (apache#3628) * Install protobuf@21 from a local tap in macOS CI Homebrew disabled protobuf@21 on 2026-01-08 and removes disabled formulae a year after their disable date. The formula still installs today with a deprecation warning; once it is removed from homebrew-core, "brew install protobuf@21" fails with "No available formula" and the compile-with-make-cmake-protobuf21 job fails. Install it from a local tap that holds the homebrew-core formula at commit cced3ef1d489 without its deprecate!/disable! lines. Homebrew pours the same bottle into the same opt/protobuf@21 prefix, so the make and cmake steps are unchanged. * Keep the protobuf@21 formula in homebrew-formula/ Store the pinned homebrew-core formula (commit cced3ef1d489, without its deprecate!/disable! lines) as homebrew-formula/protobuf@21.rb and copy it into the local tap, so the macOS job no longer downloads it on every run. The file keeps Homebrew's BSD 2-Clause notice; it is listed in LICENSE and excluded from the Apache header check, like the other third-party files.
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.
Summary
Resolve the conflict with Apache master at b4e149f after the upstream CMake module ownership refactor.
Integration
Target transport-handshake. Use a merge commit, not squash or rebase-merge, to retain the synchronized upstream master ancestry. This does not directly rewrite the community PR source branch; after integration, re-check apache/brpc pull request 3535 mergeability and CI.
Validation
Validation completed in a fresh Linux CMake build directory with RDMA and UBRing enabled:
The root CMake file, module aggregation, and test CMake structure match current upstream. The only additional CMake adaptation relative to upstream is the relocated RDMA handshake proto entry in src/brpc/CMakeLists.txt. No existing unit-test body is modified by this repair.