Replace custom timer with bthread timer and improve lifecycle safety - #3509
Replace custom timer with bthread timer and improve lifecycle safety#3509zchuango wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the UBRing/UBShmTransport timer and teardown machinery to use bthread timers, with additional lifecycle-safety mechanisms (generation checks + anchored cleanup control objects) to prevent stale timer/callback paths from touching reused trx slots and to avoid blocking work on the timer thread.
Changes:
- Replace the custom timerfd/epoll timer subsystem with a bthread-timer-based facade (
UbrTimerStart/UbrTimerDel/UbrTimerDelAndWait) and update call sites accordingly. - Introduce explicit delayed-cleanup ownership tracking via
UbrCleanupCtlanchored inUBRingManager, plus generation checks to avoid slot-reuse races. - Reduce idle close-check polling via exponential backoff, and improve teardown ordering (e.g., stop UBS SHM cleanup timer before SDK finalize).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/brpc_ubring_unittest.cpp | Extends configuration tests to cover the new close-check backoff cap flag default. |
| src/brpc/ubshm/ubr_trx.h | Replaces fd-based timers with bthread timer handles; adds cleanup control state and makes hot counters atomic. |
| src/brpc/ubshm/ub_ring.h | Updates UBRing APIs to remove timer-fd parameters and align cleanup entry points with new lifecycle model. |
| src/brpc/ubshm/ub_ring.cpp | Implements bthread-timer scheduling, delayed cleanup ownership, close-check backoff, and atomic I/O sequencing. |
| src/brpc/ubshm/ub_ring_manager.h | Adds per-slot generation + cleanup-ctl anchoring APIs to coordinate delayed cleanup across slot reuse. |
| src/brpc/ubshm/ub_ring_manager.cpp | Implements generation-safe release and teardown-time cancellation/waiting for in-flight cleanups. |
| src/brpc/ubshm/ub_helper.cpp | Removes eager timer module init (timers become lazy via bthread facade). |
| src/brpc/ubshm/ub_endpoint.cpp | Fixes poller SID updates by replacing existing set entries on ADD/MOD. |
| src/brpc/ubshm/timer/timer_mgr.h | Defines the bthread-timer facade API and documents callback/teardown expectations. |
| src/brpc/ubshm/timer/timer_mgr.cpp | Replaces timerfd/epoll/kqueue implementation with bthread timer task management and delete/wait semantics. |
| src/brpc/ubshm/shm/shm_ubs.cpp | Moves UBS cleanup work outside locks, switches cleanup timer to bthread, and fixes teardown ordering vs SDK finalize. |
| src/brpc/ubshm/common/common.h | Adds SEC_TO_USEC for consistent time conversions used by the new timer paths. |
| docs/en/ubring.md | Updates timer documentation to reflect bthread timer usage and backoff policy. |
| docs/cn/ubring.md | Same as English docs: updates timer documentation to reflect new bthread timer approach. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Non-blocking delete, safe from inside the timer callback itself. Does | ||
| // not wait for a running callback and does not protect `arg' on its own. | ||
| // Returns 0 when the call won the slot competition: a one-shot callback | ||
| // is guaranteed never to run, and the caller consumes any per-task | ||
| // resources it tracks for this timer (ownership of them transfers to the | ||
| // caller); for a periodic timer an already-started callback is not | ||
| // interrupted. Returns 1 when the callback has been dispatched (it | ||
| // consumes those resources itself on every exit) or its fate is still | ||
| // being settled by the scheduler -- the caller must not consume anything | ||
| // then. |
|
I've updated the timer deletion contract to clarify the one-shot and periodic timer semantics, and addressed the related comments. @wwbmmm could you please take another look when you have time? |
What problem does this PR solve?
Issue Number: #3463(Phase2)
Problem Summary:
This PR addresses the Phase 2 timer and lifecycle stability improvements for UBShmTransport/UBRing discussed in #3463.
The existing UBRing timer and teardown implementation has several lifecycle and scalability issues, including:
UbrTrxrelease and slot reuse;What is changed and the side effects?
Changed:
bthread_timer_add/bthread_timer_del.UbrTimerDelAndWaitfor teardown paths that need to wait for in-flight callbacks.UbrCleanupCtlto coordinate delayed cleanup and force-close ownership independently of pooledUbrTrxobjects.ub_flying_io_timeout_ssleep from the timer callback and use delayed cleanup instead.shm_lockand stop the cleanup timer beforeubsmem_finalize.Side effects:
Performance effects:
Breaking backward compatibility:
Check List:
brpc_ubring_unittestpasses.environment.