Skip to content

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

Open
zchuango wants to merge 2 commits into
apache:masterfrom
LinQuickDev:ubshm_transport_fix_2
Open

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

Conversation

@zchuango

Copy link
Copy Markdown
Contributor

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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment on lines +233 to 241
bthread_timer_t id = task->id.load();
if (id != 0 && bthread_timer_del(id) == 0) {
ReleaseRef(task); // schedule: cancelled before dispatch
} // ==1: dispatched, OnFire (owned==false)
// releases it
ReleaseRef(task); // owner
return 0; // the callback is guaranteed never to run: the caller
// consumes the timer/callback reference
}
Comment on lines +48 to +57
// 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants