Replace custom timer with bthread timer and improve lifecycle safety by zchuango · Pull Request #3509 · apache/brpc

zchuango · 2026-08-31T06:54:45Z

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:

a custom timerfd/epoll-based timer subsystem with per-fd state;
stale fd / fd reuse risks and races between timer callbacks and deletion;
blocking cleanup work running on the timer thread;
cleanup races with UbrTrx release and slot reuse;
SHM teardown ordering issues.

What is changed and the side effects?

Changed:

Replace the custom timerfd + epoll/kqueue implementation with bthread_timer_add / bthread_timer_del.
Introduce explicit timer task lifetime management and UbrTimerDelAndWait for teardown paths that need to wait for in-flight callbacks.
Introduce UbrCleanupCtl to coordinate delayed cleanup and force-close ownership independently of pooled UbrTrx objects.
Add generation checks to prevent stale cleanup paths from affecting reused trx slots.
Remove the blocking ub_flying_io_timeout_s sleep from the timer callback and use delayed cleanup instead.
Move SHM cleanup SDK calls outside shm_lock and stop the cleanup timer before ubsmem_finalize.
Add exponential backoff for idle close-check polling.
Make concurrently accessed UBRing I/O sequence counters atomic.
Update the related UBRing documentation and clean up obsolete timer code.

Side effects:

Performance effects:
- Removes the per-fd timer context table and dedicated timer thread.
- Reduces close-check polling while links are idle.
- Avoids blocking the timer thread during delayed cleanup.
Breaking backward compatibility:

Check List:

Please make sure your changes are compilable.
- CI passes, including Linux Build and Test, macOS Build, and License Check.
When providing us with a new feature, it is best to add related tests.
- brpc_ubring_unittest passes.
- Client/master communication was verified successfully in a real UB
  environment.
Please follow Contributor Covenant Code of Conduct.

Copilot

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 UbrCleanupCtl anchored in UBRingManager, 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.


zchuango · 2026-09-01T06:20:13Z

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?

wwbmmm · 2026-09-05T07:32:25Z

LGTM

zchuango · 2026-09-18T08:13:17Z

@wwbmmm Thanks for the LGTM! If there are no further concerns, I'll go ahead and merge this PR.

zchuango · 2026-09-23T02:42:26Z

@chenBright Do you have any further concerns about this PR? If not, I'll go ahead and merge it.

chenBright · 2026-09-23T02:47:37Z

-    if (ret != UBSM_OK) {
-        LOG(ERROR) << "Ubs shm finalize fail, ret=" << ret;
+    // Stop the cleanup timer before finalizing the SDK it calls into.
+    if (UNLIKELY(DestroyShmTimer(g_shm_list) != UBRING_OK)) {


UNLIKELY -> BAIDU_UNLIKELY

chenBright · 2026-09-23T02:48:09Z

+    // a slow daemon cannot stall the timer thread for the whole list.
+    SHM shm;
+    {
+        LOCK_GUARD(shm_list->shm_lock);


LOCK_GUARD -> BAIDU_SCOPED_LOCK

chenBright · 2026-09-23T02:49:24Z

+        LOG(ERROR) << "Ubs unmap shm=" << shm.name << " length=" << shm.len << " failed, ret=" << ret;
+        return nullptr;                  // node stays at head, retried
+    }
+    LOG(INFO) << "Ubs unmap shm=" << shm.name << " length=" << shm.len << " success.";


Is this INFO log necessary?

chenBright · 2026-09-23T02:49:33Z

+        LOG(ERROR) << "Ubs delete shm=" << shm.name << " failed, ret=" << ret;
+        return nullptr;
+    }
+    LOG(INFO) << "Ubs free local shm=" << shm.name << " length=" << shm.len << " success.";


Is this INFO log necessary?

chenBright · 2026-09-23T02:52:08Z

+    // the slot is the single arbiter.
+    UbrTimerId expected = task;
+    const bool owned =
+        __atomic_compare_exchange_n(task->slot, &expected, (UbrTimerId) nullptr,


Why not use butil::atomic?

chenBright · 2026-09-23T02:52:35Z

+// free the task while a callback or the starter still touches it.
+struct UbrTimerTask {
+    UbrTimerId* slot;
+    std::atomic<bthread_timer_t> id;


std::atomic -> butil::atomic

zchuango · 2026-09-23T13:08:37Z

@chenBright Thanks for the review! I'll address the comments and update the PR.

) Make the whole ubring shared-memory module follow the current brpc conventions, addressing review feedback: - Replace the module-local LIKELY/UNLIKELY aliases with BAIDU_LIKELY/ BAIDU_UNLIKELY and drop the aliases from common.h. - Use BAIDU_SCOPED_LOCK instead of the local GNU cleanup-attribute LOCK_GUARD. The pthread_mutex_t members keep their type, allocation and lifetime, so no static-init or placement-new change is needed. - Use butil::atomic instead of std::atomic. The timer handle slots and the trx cleanup slot are now real butil::atomic objects, so plain storage is never reinterpreted as an atomic: the timer API takes butil::atomic<UbrTimerId>* and UbrTrx/UbrCleanupCtl hold atomic handles. UbrTrx slots are value-initialized with placement new so those atomics are constructed, replacing the per-acquisition memset. - Drop the two per-node INFO logs on the shm cleanup retry path in UbsShmCallback; DeleteShmToList still records each drained node. No behavioral change: the removed aliases expanded 1:1, BAIDU_SCOPED_LOCK locks the same pthread_mutex_t, butil::atomic is layout-identical to std::atomic, and placement-new value-initialization zeroes exactly what memset zeroed while also constructing the atomic members.

Sherlock0203 and others added 2 commits August 31, 2026 10:42

[fix]1、unordered_set.emplace error 2、使用bthread代替原有timerfd+epoll (#33)

dbc3fee

Merge branch 'apache:master' into ubshm_transport_fix_2

44b04e6

wwbmmm requested a lite review from Copilot August 31, 2026 08:29

Copilot started reviewing on behalf of wwbmmm August 31, 2026 08:32 View session

Copilot AI reviewed Aug 31, 2026

View reviewed changes

[fix]注释修改

cdb0f95

chenBright reviewed Sep 23, 2026

View reviewed changes

Navigation Menu

Replace custom timer with bthread timer and improve lifecycle safety - #3509

Replace custom timer with bthread timer and improve lifecycle safety#3509
zchuango wants to merge 4 commits into
apache:masterfrom
LinQuickDev:ubshm_transport_fix_2

zchuango commented Aug 31, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

zchuango commented Sep 1, 2026

Uh oh!

wwbmmm commented Sep 5, 2026

Uh oh!

zchuango commented Sep 18, 2026

Uh oh!

zchuango commented Sep 23, 2026

Uh oh!

chenBright Sep 23, 2026

Uh oh!

chenBright Sep 23, 2026

Uh oh!

chenBright Sep 23, 2026

Uh oh!

chenBright Sep 23, 2026

Uh oh!

chenBright Sep 23, 2026

Uh oh!

chenBright Sep 23, 2026

Uh oh!

zchuango commented Sep 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

zchuango commented Aug 31, 2026

What problem does this PR solve?

What is changed and the side effects?

Check List:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

zchuango commented Sep 1, 2026

Uh oh!

wwbmmm commented Sep 5, 2026

Uh oh!

zchuango commented Sep 18, 2026

Uh oh!

zchuango commented Sep 23, 2026

Uh oh!

chenBright Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

chenBright Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

chenBright Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

chenBright Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

chenBright Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

chenBright Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

zchuango commented Sep 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants