diff --git a/docs/cn/ubring.md b/docs/cn/ubring.md index 1db9909103..182de6cba6 100644 --- a/docs/cn/ubring.md +++ b/docs/cn/ubring.md @@ -187,7 +187,7 @@ UBRing 架构包含以下组件： ### 定时器管理 -UBRing 使用高精度定时器系统 (`timer_mgr.cpp`) 进行连接管理和超时处理，支持 epoll（Linux）和 kqueue（macOS）。 +UBRing 的连接管理和超时处理基于 bthread 定时器（`bthread_timer_add`/`bthread_timer_del`，由 `timer_mgr.cpp` 封装）。非阻塞的 `UbrTimerDel` 可在回调内自删，此时句柄会保留到该回调返回，因此并发的外部 `UbrTimerDelAndWait` 一定能等到它；销毁回调参数所属资源的外部路径用 `UbrTimerDelAndWait` 等待运行中回调退出；一次性定时器触发后自动清理句柄。进入关闭处理时会带上调用方看到的代际，并在 manager 锁内原子认领槽位，因此按旧快照发起的关闭（例如共享内存故障事件回调）不会误伤释放后被复用的新连接。定时器回调运行在进程全局的 bthread 定时线程上，必须快速返回。阻塞的共享内存清理不在这里进行：延迟清理与待回收映射的回调只仲裁清理所有权，并把任务投递给专用的清理 worker bthread（`ub_cleanup_worker.cpp`），由它把 UBS SDK 的 `unmap`/`deallocate` 调用移出定时线程。对一次性定时器，`UbrTimerDel` 返回 0 还额外保证 facade 已不再访问句柄槽位所在的存储（它会等待被定时线程派发但尚未竞争槽位的 wrapper 退出竞争），因此调用方可以释放承载该槽位的对象；延迟清理路径正是据此释放 `UbrCleanupCtl`。关闭检查定时器在链路空闲时按指数退避轮询（上限 `ub_event_queue_timer_interval_max_us`，默认 10ms），有流量或正在关闭时恢复快速轮询。 ## 参考资料 diff --git a/docs/en/ubring.md b/docs/en/ubring.md index fbf87fff36..8cb64df6dc 100644 --- a/docs/en/ubring.md +++ b/docs/en/ubring.md @@ -186,7 +186,7 @@ The shared memory manager (`shm_mgr.cpp`) provides a unified interface for diffe ### Timer Management -UBRing uses a high-precision timer system (`timer_mgr.cpp`) for connection management and timeout handling, supporting both epoll (Linux) and kqueue (macOS). +UBRing connection management and timeout handling are built on bthread timers (`bthread_timer_add`/`bthread_timer_del`, wrapped in `timer_mgr.cpp`). The non-blocking `UbrTimerDel` is safe to call from within a timer callback; a periodic callback that deletes its own timer keeps its handle anchored until the callback returns, so a concurrent `UbrTimerDelAndWait` from a teardown path always waits for it. External teardown that frees resources reachable from the callback argument uses `UbrTimerDelAndWait` to wait out a possibly running callback; one-shot timers clean up their handle automatically when they fire. Entering a close carries the generation the caller saw and claims the pool slot atomically under the manager lock, so a close started from a stale snapshot (the shared-memory fault event, for instance) cannot hit a connection that reused the slot in the meantime. Timer callbacks run on the process-wide bthread timer thread and must return quickly. Blocking shared-memory cleanup is therefore not done there: the delayed-clear and pending-unmap callbacks only settle the cleanup ownership and post a job to a dedicated cleanup worker bthread (`ub_cleanup_worker.cpp`), which runs the UBS SDK `unmap`/`deallocate` calls off the timer thread. For a one-shot timer, a `UbrTimerDel` that returns 0 additionally guarantees that the facade is done with the storage the handle slot lives in -- it waits out a wrapper the timer thread already dispatched -- so the caller may free the object that holds the slot; the delayed-clear path frees `UbrCleanupCtl` that way. The close-check timer backs off exponentially while the link is idle (capped by `ub_event_queue_timer_interval_max_us`, 10ms by default) and returns to fast polling once there is traffic or a close in progress. ## References diff --git a/src/brpc/policy/remote_file_naming_service.cpp b/src/brpc/policy/remote_file_naming_service.cpp index b2e929544b..a2a83fe4fb 100644 --- a/src/brpc/policy/remote_file_naming_service.cpp +++ b/src/brpc/policy/remote_file_naming_service.cpp @@ -67,7 +67,12 @@ int RemoteFileNamingService::GetServers(const char *service_name_cstr, butil::StringPiece proto; if (pos != butil::StringPiece::npos) { proto = tmpname.substr(0, pos); - for (pos += 3; tmpname[pos] == '/'; ++pos) {} + for (pos += 3; pos < tmpname.size() && tmpname[pos] == '/'; ++pos) {} + if (pos == tmpname.size()) { + LOG(ERROR) << "No server address in service_name=" + << service_name_cstr; + return -1; + } tmpname.remove_prefix(pos); } else { proto = "http"; diff --git a/src/brpc/ubshm/common/common.h b/src/brpc/ubshm/common/common.h index c64ae9843f..56d2cb8706 100644 --- a/src/brpc/ubshm/common/common.h +++ b/src/brpc/ubshm/common/common.h @@ -24,9 +24,6 @@ #include "butil/compiler_specific.h" #include "butil/logging.h" -#define LIKELY(x) BAIDU_LIKELY(x) -#define UNLIKELY(x) BAIDU_UNLIKELY(x) - #ifndef UNREFERENCE_PARAM #define UNREFERENCE_PARAM(x) ((void)(x)) #endif @@ -42,11 +39,11 @@ #endif #ifdef __cplusplus -#include -using AtomicInt = std::atomic; -using AtomicBool = std::atomic; -using AtomicUintFast64 = std::atomic; -using AtomicUintFast8 = std::atomic; +#include "butil/atomicops.h" +using AtomicInt = butil::atomic; +using AtomicBool = butil::atomic; +using AtomicUintFast64 = butil::atomic; +using AtomicUintFast8 = butil::atomic; #define ATOMIC_INIT(var, value) var.store(value) #define ATOMIC_STORE(var, value) var.store(value) #define ATOMIC_LOAD(var) var.load() @@ -116,6 +113,7 @@ static inline int Copy64Byte(int8_t *dst, int8_t *src) { #define SEC_TO_NSEC 1000000000 #define MSEC_TO_NSEC 1000000 +#define SEC_TO_USEC 1000000 #define USEC_TO_NSEC 1000 #define MSEC_TO_SEC 1000 #define MAX_IP_PORT_STR_LEN 23 diff --git a/src/brpc/ubshm/common/thread_lock.h b/src/brpc/ubshm/common/thread_lock.h index 0233955168..bba5f80702 100644 --- a/src/brpc/ubshm/common/thread_lock.h +++ b/src/brpc/ubshm/common/thread_lock.h @@ -30,7 +30,7 @@ extern "C" { static inline void UnlockMutex(pthread_mutex_t **mtx) { - if (LIKELY(mtx != nullptr && *mtx != nullptr)) { + if (BAIDU_LIKELY(mtx != nullptr && *mtx != nullptr)) { pthread_mutex_unlock(*mtx); } else { LOG(ERROR) << "Invalid input for mtx."; diff --git a/src/brpc/ubshm/shm/shm_mgr.cpp b/src/brpc/ubshm/shm/shm_mgr.cpp index d4096bc3a6..3a493ded04 100644 --- a/src/brpc/ubshm/shm/shm_mgr.cpp +++ b/src/brpc/ubshm/shm/shm_mgr.cpp @@ -57,7 +57,7 @@ static bool CheckInputShmParam(SHM *shm) { } RETURN_CODE ShmMgrInit(void) { - if (UNLIKELY(FLAGS_ub_shm_type >= (int32_t)SHM_TYPE_UNSUPPORT || FLAGS_ub_shm_type <= (int32_t)SHM_TYPE_UB)) { + if (BAIDU_UNLIKELY(FLAGS_ub_shm_type >= (int32_t)SHM_TYPE_UNSUPPORT || FLAGS_ub_shm_type <= (int32_t)SHM_TYPE_UB)) { LOG(ERROR) << "Shm type config=" << FLAGS_ub_shm_type << " is not supported."; return UBRING_ERR; } @@ -92,7 +92,7 @@ SHM_TYPE GetShmType() { } RETURN_CODE ShmLocalMalloc(SHM *shm) { - if (UNLIKELY(!CheckInputShmParam(shm))) { + if (BAIDU_UNLIKELY(!CheckInputShmParam(shm))) { LOG(ERROR) << "Input param shm is invalid."; return SHM_ERR_INPUT_INVALID; } @@ -114,11 +114,11 @@ RETURN_CODE ShmLocalMalloc(SHM *shm) { RETURN_CODE ShmLocalCalloc(SHM *shm) { RETURN_CODE rc = ShmLocalMalloc(shm); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Failed to alloc local shm."; return rc; } - if (UNLIKELY(shm->addr == nullptr)) { + if (BAIDU_UNLIKELY(shm->addr == nullptr)) { LOG(ERROR) << "Local shm=" << shm->name << " allocated with NULL address."; ShmFree(shm); return SHM_ERR; @@ -128,7 +128,7 @@ RETURN_CODE ShmLocalCalloc(SHM *shm) { } RETURN_CODE ShmLocalFree(SHM *shm) { - if (UNLIKELY(!CheckInputShmParam(shm))) { + if (BAIDU_UNLIKELY(!CheckInputShmParam(shm))) { LOG(ERROR) << "Input param shm is invalid."; return SHM_ERR_INPUT_INVALID; } @@ -149,7 +149,7 @@ RETURN_CODE ShmLocalFree(SHM *shm) { } RETURN_CODE ShmRemoteMalloc(SHM *shm) { - if (UNLIKELY(!CheckInputShmParam(shm))) { + if (BAIDU_UNLIKELY(!CheckInputShmParam(shm))) { LOG(ERROR) << "Input param shm is invalid."; return SHM_ERR_INPUT_INVALID; } @@ -170,7 +170,7 @@ RETURN_CODE ShmRemoteMalloc(SHM *shm) { } RETURN_CODE ShmRemoteFree(SHM *shm) { - if (UNLIKELY(!CheckInputShmParam(shm))) { + if (BAIDU_UNLIKELY(!CheckInputShmParam(shm))) { LOG(ERROR) << "Input param shm is invalid."; return SHM_ERR_INPUT_INVALID; } @@ -191,7 +191,7 @@ RETURN_CODE ShmRemoteFree(SHM *shm) { } RETURN_CODE ShmLocalMmap(SHM *shm, int prot) { - if (UNLIKELY(!CheckInputShmParam(shm))) { + if (BAIDU_UNLIKELY(!CheckInputShmParam(shm))) { LOG(ERROR) << "Input param shm is invalid."; return SHM_ERR_INPUT_INVALID; } @@ -212,7 +212,7 @@ RETURN_CODE ShmLocalMmap(SHM *shm, int prot) { } RETURN_CODE ShmMunmap(SHM *shm) { - if (UNLIKELY(!CheckInputShmParam(shm))) { + if (BAIDU_UNLIKELY(!CheckInputShmParam(shm))) { LOG(ERROR) << "Input param shm is invalid."; return SHM_ERR_INPUT_INVALID; } @@ -233,7 +233,7 @@ RETURN_CODE ShmMunmap(SHM *shm) { } RETURN_CODE ShmFree(SHM *shm) { - if (UNLIKELY(!CheckInputShmParam(shm))) { + if (BAIDU_UNLIKELY(!CheckInputShmParam(shm))) { LOG(ERROR) << "Input param shm is invalid."; return SHM_ERR_INPUT_INVALID; } diff --git a/src/brpc/ubshm/shm/shm_ubs.cpp b/src/brpc/ubshm/shm/shm_ubs.cpp index ccbd325923..699c26dd44 100644 --- a/src/brpc/ubshm/shm/shm_ubs.cpp +++ b/src/brpc/ubshm/shm/shm_ubs.cpp @@ -22,8 +22,9 @@ #include #include #include +#include "butil/scoped_lock.h" #include "brpc/ubshm/timer/timer_mgr.h" -#include "brpc/ubshm/common/thread_lock.h" +#include "brpc/ubshm/ub_cleanup_worker.h" #include "brpc/ubshm/common/common.h" #include "brpc/ubshm/shm/shm_def.h" #include "brpc/ubshm/ub_ring_manager.h" @@ -47,8 +48,17 @@ DEFINE_int32(ub_flying_io_timeout_s, 5, "Time in seconds to wait for stopping data sending and receiving " "when the link is disconnected."); char g_region_name[MAX_REGION_NAME_DESC_LENGTH] = {0}; -int g_shm_timer_fd = 0; +butil::atomic g_shm_timer_id(nullptr); ShmList *g_shm_list = nullptr; +// True while a drain step of g_shm_list is queued on (or running in) the +// cleanup worker. It bounds the job backlog: the periodic timer only posts a +// new step when the previous one finished, and the worker clears the flag at +// the end of every step, successful or not. +static butil::atomic g_shm_drain_pending(false); +// Set by UbsShmFini, cleared by UbsShmInit: ShmMgrFini may run more than once, +// and the SDK finalize plus the shm list teardown must happen exactly once per +// init. +static bool g_ubs_shm_finalized = false; static RETURN_CODE UbsShmInterfacesLoad(void); char hostname[MAX_HOST_NAME_DESC_LENGTH]; @@ -314,6 +324,9 @@ void UbsMemLoggerPrint(int level, const char *msg) RETURN_CODE UbsShmInit(void) { + // A new init makes UbsShmFini run its teardown again. + g_ubs_shm_finalized = false; + // load libubsm_sdk.so and get function pointer RETURN_CODE ret_code = UbsShmInterfacesLoad(); if (ret_code != UBRING_OK) { @@ -351,12 +364,12 @@ RETURN_CODE UbsShmInit(void) return UBRING_ERR; } - if (UNLIKELY(ubsmem_local_nid_query(&FLAGS_node_location) != UBSM_OK)) { + if (BAIDU_UNLIKELY(ubsmem_local_nid_query(&FLAGS_node_location) != UBSM_OK)) { LOG(ERROR) << "Get local nid failed."; return UBRING_ERR; } - if (UNLIKELY(ubsmem_shmem_faults_register(brpc::ubring::UBRingManager::UbEventCallback) != UBSM_OK)) { + if (BAIDU_UNLIKELY(ubsmem_shmem_faults_register(brpc::ubring::UBRingManager::UbEventCallback) != UBSM_OK)) { LOG(ERROR) << "Failed to register the ub event callback function."; return UBRING_ERR; } @@ -377,17 +390,26 @@ RETURN_CODE UbsShmInit(void) RETURN_CODE UbsShmFini(void) { - int ret = ubsmem_finalize(); - if (ret != UBSM_OK) { - LOG(ERROR) << "Ubs shm finalize fail, ret=" << ret; - return UBRING_ERR; + // Idempotent: ShmMgrFini is allowed to run more than once (GlobalRelease + // plus an explicit caller), and finalizing the SDK or tearing the shm list + // down twice would touch freed memory. + if (g_ubs_shm_finalized) { + return UBRING_OK; } + g_ubs_shm_finalized = true; - if (UNLIKELY(DestroyShmTimer(g_shm_list) != UBRING_OK)) { + // Stop the cleanup timer before finalizing the SDK it calls into. + if (BAIDU_UNLIKELY(DestroyShmTimer(&g_shm_list) != UBRING_OK)) { LOG(ERROR) << "Ubs shm list finalize failed."; return UBRING_ERR; } + int ret = ubsmem_finalize(); + if (ret != UBSM_OK) { + LOG(ERROR) << "Ubs shm finalize fail, ret=" << ret; + return UBRING_ERR; + } + LOG(INFO) << "Ubs shm finalize success."; return UBRING_OK; } @@ -410,58 +432,105 @@ static void DeleteShmToList(ShmList* shm_list) shm_list->size--; } -void *UbsShmCallback(void* args) +void *UbsShmCallback(void* args, uint64_t) { ShmList *shm_list = (ShmList*)args; - if (UNLIKELY(shm_list == nullptr)) { + if (BAIDU_UNLIKELY(shm_list == nullptr)) { LOG(ERROR) << "Shm list is null."; return nullptr; } - LOCK_GUARD(shm_list->shm_lock); - while (shm_list->head != nullptr) { - SHM shm = shm_list->head->shm; - if (shm.addr == nullptr) { - LOG(ERROR) << "Ubs input shm param is invalid, addr is NULL."; - return nullptr; + // The timer thread runs every timer in the process, so it must not call + // into the SDK: post the drain to the cleanup worker. At most one step may + // be in flight, otherwise a slow SDK call would pile up jobs for the same + // list head. + if (g_shm_drain_pending.exchange(true)) { + return nullptr; + } + if (BAIDU_UNLIKELY(!UbrCleanupWorker::PostShmDrain(shm_list, UbsShmDrainNode))) { + // The worker could not be started: keep the pre-worker behaviour and + // drain here rather than leaving the nodes in the list forever. + LOG(ERROR) << "Cleanup worker unavailable, draining shm on the timer thread."; + UbsShmDrainNode(shm_list); + } + return nullptr; +} + +void UbsShmDrainNode(ShmList *shm_list) +{ + if (BAIDU_UNLIKELY(shm_list == nullptr)) { + LOG(ERROR) << "Shm list is null."; + g_shm_drain_pending.store(false); + return; + } + + // Drain one node per step and keep the SDK calls outside the lock, so a + // slow daemon cannot stall other users of the list. + SHM shm; + { + BAIDU_SCOPED_LOCK(shm_list->shm_lock); + if (shm_list->head == nullptr) { + g_shm_drain_pending.store(false); + return; } + shm = shm_list->head->shm; + } + if (shm.addr == nullptr) { + LOG(ERROR) << "Ubs input shm param is invalid, addr is NULL."; + BAIDU_SCOPED_LOCK(shm_list->shm_lock); + DeleteShmToList(shm_list); + g_shm_drain_pending.store(false); + return; + } - int ret = ubsmem_shmem_unmap(shm.addr, shm.len); - if (ret != UBSM_OK) { - if (ret == UBSM_ERR_NET) { - return nullptr; - } + int ret = ubsmem_shmem_unmap(shm.addr, shm.len); + if (ret != UBSM_OK) { + if (ret != UBSM_ERR_NET) { LOG(ERROR) << "Ubs unmap shm=" << shm.name << " length=" << shm.len << " failed, ret=" << ret; - return nullptr; } - LOG(INFO) << "Ubs unmap shm=" << shm.name << " length=" << shm.len << " success."; + // The node stays at the head: the next timer fire retries it. + g_shm_drain_pending.store(false); + return; + } - ret = ubsmem_shmem_deallocate(shm.name); - if (ret != UBSM_OK) { - DeleteShmToList(shm_list); - LOG(ERROR) << "Ubs delete shm=" << shm.name << " failed, ret=" << ret; - return nullptr; - } + { + BAIDU_SCOPED_LOCK(shm_list->shm_lock); DeleteShmToList(shm_list); - LOG(INFO) << "Ubs free local shm=" << shm.name << " length=" << shm.len << " success."; } - return nullptr; + ret = ubsmem_shmem_deallocate(shm.name); + if (ret != UBSM_OK) { + LOG(ERROR) << "Ubs delete shm=" << shm.name << " failed, ret=" << ret; + } + g_shm_drain_pending.store(false); } RETURN_CODE UbsShmAddTimer(ShmList *shm_list) { - const uint32_t timer_interval_s = FLAGS_ub_flying_io_timeout_s; - itimerspec time_spec = { - .it_interval = {.tv_sec = timer_interval_s, .tv_nsec = 0}, - .it_value = {.tv_sec = 0, .tv_nsec = 1} - }; - int timer_fd = TimerStart(&time_spec, UbsShmCallback, (void*)shm_list); - if (UNLIKELY(timer_fd == -1)) { + // This timer is periodic -- it drains one pending unmap per fire -- while + // the same flag is also the one-shot delay of the delayed-clear path, where + // 0 legitimately means "do not wait for in-flight IO". A periodic timer + // needs a positive period, so fall back to a drain period of one second + // instead of either refusing to start (which would break UBS shm init for a + // valid tuning of the other use) or degrading to a single drain. + uint64_t period_s = (uint64_t)FLAGS_ub_flying_io_timeout_s; + if (BAIDU_UNLIKELY(FLAGS_ub_flying_io_timeout_s <= 0)) { + LOG(WARNING) << "ub_flying_io_timeout_s=" << FLAGS_ub_flying_io_timeout_s + << " disables the delayed-clear wait; using a 1s drain period " + << "for the shm cleanup timer."; + period_s = 1; + } + const uint64_t timer_interval_us = period_s * SEC_TO_USEC; + // The generation argument is the caller's staleness guard; UbsShmCallback + // ignores it and never frees `shm_list', whose lifetime is settled by + // DestroyShmTimer waiting through UbrTimerDelAndWait before it tears the + // list down. 0 is therefore deliberate here, not a missing generation. + RETURN_CODE rc = UbrTimerStartPeriodic(&g_shm_timer_id, 0, timer_interval_us, + UbsShmCallback, (void*)shm_list, 0); + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Start shm timer failed."; return UBRING_ERR; } - g_shm_timer_fd = timer_fd; return UBRING_OK; } @@ -491,14 +560,26 @@ RETURN_CODE InitShmTimer(ShmList **shm_list) return UBRING_OK; } -RETURN_CODE DestroyShmTimer(ShmList *shm_list) +RETURN_CODE DestroyShmTimer(ShmList **shm_list) { - DeleteTimerSafe((uint32_t)g_shm_timer_fd); - if (shm_list == nullptr) { - LOG(WARNING) << "Shm list is null."; + // Wait out a possibly running UbsShmCallback, then the cleanup jobs it + // posted: the worker may be inside the SDK calls of a node that is freed + // right below, and must not race ubsmem_finalize either. + UbrTimerDelAndWait(&g_shm_timer_id); + UbrCleanupWorker::DrainAndWait(); + if (BAIDU_UNLIKELY(shm_list == nullptr)) { + LOG(ERROR) << "Shm list handle is null."; return UBRING_ERR; } - ShmListNode* current = shm_list->head; + if (*shm_list == nullptr) { + return UBRING_OK; // already destroyed, idempotent + } + + // Own the pointer for the whole teardown, and clear the caller's handle + // (FREE_PTR does that for us) before returning: leaving it dangling lets a + // second ShmMgrFini/AddShmToList use freed memory. + ShmList* list = *shm_list; + ShmListNode* current = list->head; ShmListNode* next; while (current != nullptr) { @@ -506,18 +587,19 @@ RETURN_CODE DestroyShmTimer(ShmList *shm_list) free(current); current = next; } - pthread_mutex_destroy(&shm_list->shm_lock); - FREE_PTR(shm_list); + pthread_mutex_destroy(&list->shm_lock); + FREE_PTR(*shm_list); + LOG(INFO) << "Ubs shm list destroyed."; return UBRING_OK; } RETURN_CODE IsExistInShmList(ShmList *shm_list, const SHM *shm) { - if (UNLIKELY(shm_list == nullptr || shm == nullptr)) { + if (BAIDU_UNLIKELY(shm_list == nullptr || shm == nullptr)) { LOG(ERROR) << "Shm list or shm is null."; return UBRING_ERR; } - LOCK_GUARD(shm_list->shm_lock); + BAIDU_SCOPED_LOCK(shm_list->shm_lock); ShmListNode *cur_node = shm_list->head; while (cur_node != nullptr) { @@ -548,7 +630,7 @@ RETURN_CODE AddShmToList(ShmList *shm_list, SHM *shm) } memcpy(&new_shm_node->shm, shm, sizeof(SHM)); - LOCK_GUARD(shm_list->shm_lock); + BAIDU_SCOPED_LOCK(shm_list->shm_lock); new_shm_node->next = nullptr; new_shm_node->prev = shm_list->tail; if (shm_list->tail) { diff --git a/src/brpc/ubshm/shm/shm_ubs.h b/src/brpc/ubshm/shm/shm_ubs.h index ee936c0a55..562e1f52c2 100644 --- a/src/brpc/ubshm/shm/shm_ubs.h +++ b/src/brpc/ubshm/shm/shm_ubs.h @@ -17,6 +17,7 @@ #ifndef BRPC_SHM_UBS_H #define BRPC_SHM_UBS_H +#include namespace brpc { namespace ubring { DECLARE_int32(ub_flying_io_timeout_s); @@ -40,10 +41,18 @@ RETURN_CODE UbsShmFini(void); RETURN_CODE UbsShmLocalMmap(SHM *shm, int prot); void UbsMemLoggerPrint(int level, const char *msg); -void *UbsShmCallback(void* args); +void *UbsShmCallback(void* args, uint64_t gen); +// One drain step of the pending-unmap list: unmap the head node and deallocate +// it. Called by the cleanup worker (and inline by UbsShmCallback when that +// worker cannot be started), never by the timer thread directly. The head node +// stays in the list when the unmap fails, so a later timer fire retries it. +void UbsShmDrainNode(ShmList *shm_list); RETURN_CODE UbsShmAddTimer(ShmList *shm_list); RETURN_CODE InitShmTimer(ShmList **shm_list); -RETURN_CODE DestroyShmTimer(ShmList *shm_list); +// Takes the handle so it can be cleared after the list is freed: a caller that +// keeps the pointer (g_shm_list) must not be left with a dangling one. Calling +// it again on a cleared handle is a no-op. +RETURN_CODE DestroyShmTimer(ShmList **shm_list); RETURN_CODE AddShmToList(ShmList *shm_list, SHM *shm); RETURN_CODE IsExistInShmList(ShmList *shm_list, const SHM *shm); } diff --git a/src/brpc/ubshm/timer/timer_mgr.cpp b/src/brpc/ubshm/timer/timer_mgr.cpp index b5e0c9ef3b..8eb864dd00 100644 --- a/src/brpc/ubshm/timer/timer_mgr.cpp +++ b/src/brpc/ubshm/timer/timer_mgr.cpp @@ -15,454 +15,378 @@ // specific language governing permissions and limitations // under the License. -#define _GNU_SOURCE -#include -#include -#include -#include -#include -#include -#include -#include +#include +#include "bthread/bthread.h" // bthread_usleep +#include "bthread/unstable.h" // bthread_timer_add/del +#include "butil/atomicops.h" +#include "butil/time.h" #include "brpc/ubshm/timer/timer_mgr.h" namespace brpc { namespace ubring { -int32_t g_epoll_fd = -1; -std::atomic g_total_timer_num(0); -TimerFdCtx *g_timer_fd_ctx_map = nullptr; -uint32_t g_max_system_fd = 0; -static pthread_t g_epoll_execute_thread = 0; -static int32_t g_timer_module_initialized = 0; - -#if defined(OS_MACOSX) -static int timerfd_create_macosx(int clockid, int flags); -static int timerfd_settime_macosx(int fd, int flags, - const itimerspec *new_value, - itimerspec *old_value); -#endif - -static RETURN_CODE DeleteTimerInner(uint32_t fd) { - if (g_timer_fd_ctx_map == nullptr) { - return UBRING_OK; - } - - if (pthread_spin_lock(&g_timer_fd_ctx_map[fd].spin_lock) != 0) { - return UBRING_ERR; - } - - if (g_timer_fd_ctx_map[fd].status == TIMER_CONTEXT_NOT_USING) { - pthread_spin_unlock(&g_timer_fd_ctx_map[fd].spin_lock); - return UBRING_OK; - } - - g_timer_fd_ctx_map[fd].status = TIMER_CONTEXT_NOT_USING; - g_timer_fd_ctx_map[fd].cb = nullptr; - g_timer_fd_ctx_map[fd].args = nullptr; - g_timer_fd_ctx_map[fd].periodical = 0; - g_timer_fd_ctx_map[fd].fd = 0; - - pthread_spin_unlock(&g_timer_fd_ctx_map[fd].spin_lock); - -#if defined(OS_LINUX) - epoll_ctl(g_epoll_fd, EPOLL_CTL_DEL, (int)fd, nullptr); -#elif defined(OS_MACOSX) - struct kevent evt; - EV_SET(&evt, fd, EVFILT_TIMER, EV_DELETE, 0, 0, nullptr); - kevent(g_epoll_fd, &evt, 1, nullptr, 0, nullptr); -#endif - - uint64_t exp = 0; - read((int)fd, &exp, sizeof(exp)); - - close((int)fd); - std::atomic_fetch_sub(&g_total_timer_num, 1U); - return UBRING_OK; -} - -static RETURN_CODE StartTimeEpoll(void) { -#if defined(OS_LINUX) - g_epoll_fd = epoll_create1(0); -#elif defined(OS_MACOSX) - g_epoll_fd = kqueue(); -#endif - if (UNLIKELY(g_epoll_fd == -1)) { - LOG(ERROR) << "Failed to create epoll/kqueue. errno=" << errno; - return UBRING_ERR; - } - - int ret = pthread_create(&g_epoll_execute_thread, nullptr, TimerEpoll, nullptr); - if (UNLIKELY(ret != 0)) { - LOG(ERROR) << "Failed to create thread err=" << ret; - return UBRING_ERR; - } - return UBRING_OK; -} - -static RETURN_CODE TimerSpinLocksInit(void) { - if (g_timer_fd_ctx_map == nullptr) { - LOG(ERROR) << "Timer module is not fully initialized."; - return UBRING_ERR; - } - - for (uint32_t fd = 0; fd < g_max_system_fd; fd++) { - int ret = pthread_spin_init(&g_timer_fd_ctx_map[fd].spin_lock, - PTHREAD_PROCESS_PRIVATE); - if (ret != EOK) { - LOG(ERROR) << "Failed to initialize spin lock for fd=" << fd; - for (uint32_t cleanup_fd = 0; cleanup_fd < fd; cleanup_fd++) { - pthread_spin_destroy(&g_timer_fd_ctx_map[cleanup_fd].spin_lock); - } - return UBRING_ERR; +namespace { + +enum UbrTimerState { + kStarting = 0, // published, not scheduled yet + kScheduled = 1, + kDead = 2 // scheduling failed +}; + +// Granularity of the two join waits in UbrTimerDelAndWait. They only run on +// teardown paths and only for the short window in which a timer is being +// started or a callback is finishing. +constexpr uint64_t kTimerPollIntervalUs = 100; + +} // namespace + +// Reference rules: one "owner" ref for the handle slot, one "schedule" ref +// per pending/running bthread schedule, plus one ref held by the starter +// until its post-schedule bookkeeping is done. The schedule ref is +// consumed by the firing callback or by the deleter whose +// bthread_timer_del returned 0 (cancelled before run); the owner ref is +// consumed by whoever takes the task out of *slot -- a deleter, or the +// one-shot firing callback itself, which exits the slot BEFORE running +// the callback so that the callback may free the object storing the slot. +// All atomics are seq_cst so no interleaving can release a ref twice or +// free the task while a callback or the starter still touches it. +// +// A periodic callback that deletes its own timer does NOT take the task out +// of the slot: it only marks the task stopped, and the firing callback +// retires the slot right before it returns (see UbrTimerOnFire). Keeping the +// task anchored for the whole callback is what lets a concurrent +// UbrTimerDelAndWait still find it and wait for the callback to finish, +// instead of returning as if no callback were in flight. +struct UbrTimerTask { + butil::atomic* slot; + butil::atomic id; + void* (*cb)(void*, uint64_t); + void* arg; + uint64_t gen; // opaque, passed back to cb + UbrTimerBackoffFn backoff; + uint64_t interval_us; // timer thread only + bool periodic; + butil::atomic state; // kStarting/kScheduled/kDead + butil::atomic stopped; + butil::atomic ref; + butil::atomic join_pending; // a DelAndWait is waiting + butil::atomic done; // refs hit zero, joiner frees + // Set by whoever settles the handle slot: the one-shot wrapper (right + // after its slot CAS, before it runs the user callback), a deleter whose + // bthread_timer_del cancelled the task before dispatch, or the kDead + // path. Once true, no facade code -- including the dispatched wrapper -- + // will ever touch `*slot` again, so the object storing the slot may be + // freed. The scheduled wrapper is kept alive by its own schedule + // reference while a deleter waits on this flag. + butil::atomic slot_retired; +}; + +namespace { + +// The task whose callback is running on this thread, if any. It lets +// UbrTimerDel recognize a delete issued by the callback's own dispatch and +// leave the task anchored until the callback returns. +thread_local UbrTimerTask* t_running_task = nullptr; + +void ReleaseRef(UbrTimerTask* task) { + if (task->ref.fetch_sub(1) == 1) { + if (task->join_pending.load()) { + task->done.store(true); // joiner frees the task + } else { + delete task; } } - return UBRING_OK; } -static RETURN_CODE ExecuteCallback(int32_t timer_fd) { - UnifiedCallback((void *)(&g_timer_fd_ctx_map[timer_fd])); - return UBRING_OK; -} - -static RETURN_CODE TimerCtxMapCompletion(void) { - memset(g_timer_fd_ctx_map, 0, sizeof(TimerFdCtx) * g_max_system_fd); - - RETURN_CODE ret = TimerSpinLocksInit(); - if (ret != UBRING_OK) { - LOG(ERROR) << "Failed to init spin locks for timer module."; - return UBRING_ERR; - } - return UBRING_OK; -} - -RETURN_CODE TimerInit(void) { - if (g_timer_module_initialized > 0) { - return UBRING_OK; - } - - g_total_timer_num.store(0); - - struct rlimit rlim; - if (getrlimit(RLIMIT_NOFILE, &rlim) != UBRING_OK) { - LOG(ERROR) << "Failed to get fd"; - return UBRING_ERR; - } - g_max_system_fd = (uint32_t)rlim.rlim_cur; - - if (g_timer_fd_ctx_map == nullptr) { - g_timer_fd_ctx_map = (TimerFdCtx *)malloc(sizeof(TimerFdCtx) * g_max_system_fd); - if (UNLIKELY(!g_timer_fd_ctx_map)) { - LOG(ERROR) << "Fail to malloc space for timer modules. errno=%d", errno; - return UBRING_ERR; - } - - RETURN_CODE ret = TimerCtxMapCompletion(); - if (ret != UBRING_OK) { - LOG(ERROR) << "Failed to init main data structure of Time Module. ret=" << ret; - free(g_timer_fd_ctx_map); - g_timer_fd_ctx_map = nullptr; - return UBRING_ERR; +void UbrTimerOnFire(void* p) { + UbrTimerTask* task = (UbrTimerTask*)p; + + if (task->periodic) { + if (!task->stopped.load()) { + // Publish the running task so an UbrTimerDel issued by this very + // callback leaves the slot anchored for a concurrent waiter. + UbrTimerTask* prev_running = t_running_task; + t_running_task = task; + task->cb(task->arg, task->gen); + t_running_task = prev_running; } - } - - RETURN_CODE ret = StartTimeEpoll(); - if (ret != UBRING_OK) { - LOG(ERROR) << "Failed to start Timer Epoll. ret=" << ret; - if (LIKELY(g_timer_fd_ctx_map != nullptr)) { - FREE_PTR(g_timer_fd_ctx_map); - } - return UBRING_ERR; - } - g_timer_module_initialized = 1; - return UBRING_OK; -} - -void *UnifiedCallback(void *args) { - TimerFdCtx *ctx = (TimerFdCtx *)args; - if (pthread_spin_lock(&ctx->spin_lock) != 0) { - return nullptr; - } - - if (ctx->status == TIMER_CONTEXT_NOT_USING) { - pthread_spin_unlock(&ctx->spin_lock); - return nullptr; - } - - void *(*cb)(void *) = ctx->cb; - void *cb_args = ctx->args; - uint32_t fd = ctx->fd; - int is_periodical = ctx->periodical; - ctx->status = TIMER_CONTEXT_CALLBACK_ONGOING; - - pthread_spin_unlock(&ctx->spin_lock); - - cb(cb_args); - - if (!is_periodical) { - DeleteTimerInner(fd); - } - return nullptr; -} - -void *TimerEpoll(void *args) { - UNREFERENCE_PARAM(args); -#if defined(OS_LINUX) - struct epoll_event ready_events[MAX_TIMER]; -#elif defined(OS_MACOSX) - struct kevent ready_events[MAX_TIMER]; -#endif - - while (1) { - if (g_timer_module_initialized <= 0) { - LOG(ERROR) << "The Timer module is not initialized."; - break; - } - -#if defined(OS_LINUX) - int32_t ready_num = epoll_wait(g_epoll_fd, ready_events, MAX_TIMER, - TIMER_EPOLL_WAIT_TIMEOUT); -#elif defined(OS_MACOSX) - struct timespec timeout = {0, TIMER_EPOLL_WAIT_TIMEOUT * 1000000}; - int32_t ready_num = kevent(g_epoll_fd, nullptr, 0, ready_events, MAX_TIMER, &timeout); -#endif - - if (UNLIKELY(ready_num == -1)) { - errno_t err = errno; - if (err == EINTR) { - LOG_EVERY_SECOND(WARNING) << "Epoll/Kqueue wait was interrupted. errno=" << err; - continue; - } else if (err == EBADF) { - LOG(WARNING) << "The Timer module is destroyed."; - break; + // Claim the next schedule's ref before re-reading `stopped' so a + // racing delete can neither free the task nor orphan a re-arm. + task->ref.fetch_add(1); + if (task->stopped.load()) { + // Retire the slot here, on the callback's own exit: a self-delete + // deliberately left the task anchored. A deleter that took the + // slot instead has already consumed the owner reference. + UbrTimerId anchored = task; + if (task->slot->compare_exchange_strong(anchored, nullptr)) { + ReleaseRef(task); // owner } - LOG(ERROR) << "Epoll/Kqueue wait internal error. errno=" << err; - break; - } - - for (int32_t i = 0; i < ready_num; i++) { -#if defined(OS_LINUX) - struct epoll_event *event = &ready_events[i]; - int32_t timer_fd = event->data.fd; -#elif defined(OS_MACOSX) - struct kevent *event = &ready_events[i]; - int32_t timer_fd = event->ident; -#endif - - uint64_t exp = 0; - if (read(timer_fd, &exp, sizeof(exp)) < 0) { - if (errno != EBADF) { - LOG(ERROR) << "Failed to read timerfd=" << timer_fd << " errno=" << errno; + ReleaseRef(task); // claimed next schedule + } else { + uint64_t interval = task->interval_us; + if (task->backoff != nullptr) { + const uint64_t next = task->backoff(task->arg, interval); + if (BAIDU_UNLIKELY(next == 0)) { + // Re-arming for "now" would let this one timer monopolize + // the process-wide timer thread. A back-off (or a flag it + // reads) returning 0 is a bug, so keep the previous + // interval instead of honouring it. Guarded here rather + // than in each back-off function, because any of them can + // make this mistake. + LOG_EVERY_SECOND(ERROR) << "Ubr timer back-off returned 0, keeping interval_us=" + << interval; + } else { + interval = next; } - continue; - } - if (TimerFdCtxValidate((uint32_t)timer_fd) != UBRING_OK) { - continue; + task->interval_us = interval; } - - RETURN_CODE ret = ExecuteCallback(timer_fd); - if (ret != UBRING_OK) { - LOG(ERROR) << "Failed execute callback ret=" << ret; - DeleteTimerInner((uint32_t)timer_fd); - continue; + bthread_timer_t id = 0; + if (bthread_timer_add( + &id, butil::microseconds_from_now((int64_t)interval), + UbrTimerOnFire, task) == 0) { + task->id.store(id); + if (task->stopped.load() && bthread_timer_del(id) == 0) { + ReleaseRef(task); + } + } else { + LOG(ERROR) << "Fail to re-arm ubring timer"; + ReleaseRef(task); } } - } - return nullptr; -} - -void DeleteTimerSafe(uint32_t fd) { - if (g_timer_fd_ctx_map == nullptr) { + ReleaseRef(task); return; } - if (pthread_spin_lock(&g_timer_fd_ctx_map[fd].spin_lock) != 0) { - return; + // One-shot: exit the handle slot first -- after this the wrapper never + // touches the storage again, so the callback may release the object + // that holds it. Whether the callback runs is decided solely by this + // slot competition: every UbrTimerDel that wants the callback + // suppressed has to win this exchange first, so owned==true guarantees + // no UbrTimerDel is pending. Do not consult `stopped' here: its store + // (del thread) and this load (timer thread) are separated by the slot + // RMW and seq_cst does not order the store-buffer case -- ownership of + // the slot is the single arbiter. + UbrTimerId expected = task; + const bool owned = task->slot->compare_exchange_strong(expected, nullptr); + // The slot storage is now retired: whatever the competition above + // decided, this wrapper will not dereference `slot' again. A one-shot + // UbrTimerDel that won the exchange (or that cancelled a task which had + // not been dispatched at all) is waiting for this store before it lets + // the caller free the storage, so it must happen before the potentially + // long user callback. + task->slot_retired.store(true); + if (owned) { + task->cb(task->arg, task->gen); + } + ReleaseRef(task); // schedule + if (owned) { + ReleaseRef(task); // owner } - - if (g_timer_fd_ctx_map[fd].status == TIMER_CONTEXT_NOT_USING) { - pthread_spin_unlock(&g_timer_fd_ctx_map[fd].spin_lock); - return; - } - - g_timer_fd_ctx_map[fd].status = TIMER_CONTEXT_NOT_USING; - g_timer_fd_ctx_map[fd].cb = nullptr; - g_timer_fd_ctx_map[fd].args = nullptr; - g_timer_fd_ctx_map[fd].periodical = 0; - g_timer_fd_ctx_map[fd].fd = 0; - - pthread_spin_unlock(&g_timer_fd_ctx_map[fd].spin_lock); - -#if defined(OS_LINUX) - epoll_ctl(g_epoll_fd, EPOLL_CTL_DEL, (int)fd, nullptr); -#elif defined(OS_MACOSX) - struct kevent evt; - EV_SET(&evt, fd, EVFILT_TIMER, EV_DELETE, 0, 0, nullptr); - kevent(g_epoll_fd, &evt, 1, nullptr, 0, nullptr); -#endif - - uint64_t exp = 0; - read((int)fd, &exp, sizeof(exp)); - - close((int)fd); - std::atomic_fetch_sub(&g_total_timer_num, 1U); } -void DeleteTimer(uint32_t fd) { - if (g_timer_fd_ctx_map == nullptr) { - LOG(WARNING) << "The timer is not initialized."; - return; - } - - g_timer_fd_ctx_map[fd].periodical = 0; +UbrTimerTask* TakeOutTask(butil::atomic* slot) { + return slot->exchange(nullptr); } -int32_t TimerStart(const itimerspec *time, void *(*cb)(void *), void *args) { - if (g_epoll_fd == -1) { - LOG(ERROR) << "Timer epoll/kqueue encountered internal error."; - return -1; +RETURN_CODE TimerStartInternal(butil::atomic* slot, uint64_t delay_us, + uint64_t interval_us, bool periodic, + void* (*cb)(void*, uint64_t), + void* arg, uint64_t gen, + UbrTimerBackoffFn backoff) { + if (BAIDU_UNLIKELY(slot == nullptr || cb == nullptr)) { + LOG(ERROR) << "Ubr timer start invalid argument, slot=" << slot; + return UBRING_ERR; } - -#if defined(OS_LINUX) - int timer_fd = timerfd_create(CLOCK_MONOTONIC, 0); -#elif defined(OS_MACOSX) - int timer_fd = timerfd_create_macosx(CLOCK_MONOTONIC, 0); -#endif - - if (UNLIKELY(timer_fd >= (int)g_max_system_fd || timer_fd == -1)) { - LOG(ERROR) << "Failed to create timerfd=" << timer_fd << " errno=" << errno; - return -1; + if (BAIDU_UNLIKELY(periodic && interval_us == 0)) { + // Rejecting beats silently arming a one-shot: a caller that asks for a + // periodic timer and gets a single fire loses whatever the timer was + // watching for the rest of the connection's life. + LOG(ERROR) << "Ubr periodic timer start requires a positive interval."; + return UBRING_ERR; } - g_timer_fd_ctx_map[timer_fd].status = TIMER_CONTEXT_EPOLL_WAITING; - g_timer_fd_ctx_map[timer_fd].cb = cb; - g_timer_fd_ctx_map[timer_fd].args = args; - g_timer_fd_ctx_map[timer_fd].fd = (uint32_t)timer_fd; - - if (LIKELY(time->it_interval.tv_sec > 0 || time->it_interval.tv_nsec > 0)) { - g_timer_fd_ctx_map[timer_fd].periodical = 1; + UbrTimerTask* task = new (std::nothrow) UbrTimerTask(); + if (BAIDU_UNLIKELY(task == nullptr)) { + LOG(ERROR) << "Fail to malloc ubring timer task."; + return UBRING_ERR; } - -#if defined(OS_LINUX) - struct epoll_event event = { - .events = EPOLLIN, - .data = {.fd = timer_fd} - }; - - int32_t ret = epoll_ctl(g_epoll_fd, EPOLL_CTL_ADD, timer_fd, &event); -#elif defined(OS_MACOSX) - struct kevent event; - uint64_t timeout_nsec = time->it_value.tv_sec * 1000000000ULL + time->it_value.tv_nsec; - uint64_t interval_nsec = time->it_interval.tv_sec * 1000000000ULL + time->it_interval.tv_nsec; - EV_SET(&event, timer_fd, EVFILT_TIMER, EV_ADD | EV_ENABLE, 0, - timeout_nsec / 1000000, nullptr); - int32_t ret = kevent(g_epoll_fd, &event, 1, nullptr, 0, nullptr); -#endif - - if (UNLIKELY(ret != 0)) { - CloseTimerFd(timer_fd); - LOG(ERROR) << "Failed to add event to epoll/kqueue. errno=" << errno; - return -1; + task->slot = slot; + task->id.store(0); + task->cb = cb; + task->arg = arg; + task->gen = gen; + task->backoff = backoff; + task->interval_us = interval_us; + task->periodic = periodic; + task->state.store(kStarting); + task->stopped.store(false); + task->ref.store(3); // owner + schedule + starter + task->join_pending.store(false); + task->done.store(false); + task->slot_retired.store(false); + + // Publish the real task before scheduling so a delete or a DelAndWait + // racing the start always has an object to act on or wait for. + UbrTimerId expected = nullptr; + if (!slot->compare_exchange_strong(expected, task)) { + LOG(ERROR) << "Ubr timer start refused, slot already occupied"; + delete task; // never published + return UBRING_ERR; } - std::atomic_fetch_add(&g_total_timer_num, 1U); - -#if defined(OS_LINUX) - ret = timerfd_settime(timer_fd, 0, time, nullptr); -#elif defined(OS_MACOSX) - ret = timerfd_settime_macosx(timer_fd, 0, time, nullptr); -#endif - - if (UNLIKELY(ret != 0)) { -#if defined(OS_LINUX) - if (epoll_ctl(g_epoll_fd, EPOLL_CTL_DEL, timer_fd, nullptr) != 0) { -#elif defined(OS_MACOSX) - struct kevent evt; - EV_SET(&evt, timer_fd, EVFILT_TIMER, EV_DELETE, 0, 0, nullptr); - if (kevent(g_epoll_fd, &evt, 1, nullptr, 0, nullptr) != 0) { -#endif - LOG(ERROR) << "Failed to delete the timer fd=" << timer_fd << " with errno=" << errno; + bthread_timer_t id = 0; + if (BAIDU_UNLIKELY(bthread_timer_add( + &id, butil::microseconds_from_now((int64_t)delay_us), + UbrTimerOnFire, task) != 0)) { + LOG(ERROR) << "Fail to add ubring timer"; + task->state.store(kDead); // wake DelAndWait waiters + expected = task; + const bool owned = slot->compare_exchange_strong(expected, nullptr); + ReleaseRef(task); // schedule, never ran + if (owned) { + ReleaseRef(task); // owner } - CloseTimerFd(timer_fd); - std::atomic_fetch_sub(&g_total_timer_num, 1U); - LOG(ERROR) << "Failed to set timer"; - return -1; + ReleaseRef(task); // starter + return UBRING_ERR; } - - return timer_fd; + // A zero-delay task may have fired and re-armed already; keep a newer + // id if so. + bthread_timer_t expected_id = 0; + task->id.compare_exchange_strong(expected_id, id); + task->state.store(kScheduled); + // No post-add stopped check here: a UbrTimerDel racing the start + // returns 1 without consuming the per-task resources, and the armed + // timer must fire so that OnFire settles the ownership protocol. + ReleaseRef(task); // starter + return UBRING_OK; } -uint32_t GetActiveTimerNum(void) { - return std::atomic_load(&g_total_timer_num); +} // namespace + +RETURN_CODE UbrTimerStart(butil::atomic* slot, uint64_t delay_us, + void* (*cb)(void*, uint64_t), void* arg, uint64_t gen) { + return TimerStartInternal(slot, delay_us, 0, false, cb, arg, gen, nullptr); } -void CloseTimerFd(int fd) { - g_timer_fd_ctx_map[fd].cb = nullptr; - g_timer_fd_ctx_map[fd].args = nullptr; - g_timer_fd_ctx_map[fd].status = TIMER_CONTEXT_NOT_USING; - g_timer_fd_ctx_map[fd].fd = 0; - g_timer_fd_ctx_map[fd].periodical = 0; - if (close((int)fd) != 0) { - LOG(ERROR) << "Failed to close timer fd=" << fd << " errno=" << errno; - return; - } +RETURN_CODE UbrTimerStartPeriodic(butil::atomic* slot, + uint64_t delay_us, uint64_t interval_us, + void* (*cb)(void*, uint64_t), void* arg, + uint64_t gen, UbrTimerBackoffFn backoff) { + return TimerStartInternal(slot, delay_us, interval_us, true, cb, arg, gen, + backoff); } -void TimerModuleDestroy(void) { - uint32_t max_fd = g_max_system_fd; - if (g_timer_fd_ctx_map) { - for (uint32_t fd = 0; fd < max_fd; fd++) { - if (g_timer_fd_ctx_map[fd].status != TIMER_CONTEXT_NOT_USING) { - DeleteTimerSafe(fd); - } +int UbrTimerDel(butil::atomic* slot) { + if (slot == nullptr) { + return 1; + } + // A periodic callback deleting its own timer must not pull the task out of + // the slot: the anchored task is what lets a concurrent UbrTimerDelAndWait + // find the running callback and wait for it. Mark it stopped instead and + // let UbrTimerOnFire retire the slot when the callback returns. Deleting a + // sibling timer from inside a callback is unaffected: the slots differ, so + // the pointer comparison below fails and the normal path runs. + if (t_running_task != nullptr && slot->load() == t_running_task) { + t_running_task->stopped.store(true); + // 0 means "the timer is stopped", not "the handle slot is now free": + // the slot is retired by UbrTimerOnFire when this callback returns, and + // a concurrent UbrTimerDelAndWait may still be joining the task. + return 0; + } + // Take the ownership of the slot first: after this exchange every + // dereference below is safe (the task cannot be freed while we hold + // the owner reference the slot used to anchor). + UbrTimerTask* task = TakeOutTask(slot); + if (task == nullptr) { + return 1; // fired and cleared its slot (callback side consumed) + // or another del won the exchange (it consumes) + } + task->stopped.store(true); // meaningful for periodic only + // A start still in flight cannot be cancelled nor dispatched yet; wait + // for the starter to settle the fate (kScheduled/kDead). Bounded: the + // starter stores the state before taking any lock our caller holds. + while (task->state.load() == kStarting) { + bthread_usleep(1000); + } + if (task->state.load() == kDead) { + // The wrapper never ran and never will: this call is the one that + // settles the slot storage. + task->slot_retired.store(true); + ReleaseRef(task); // owner; schedule/starter are + return 1; // settled by the kDead path + } + bthread_timer_t id = task->id.load(); + if (id != 0 && bthread_timer_del(id) == 0) { + // Cancelled before dispatch: the timer thread will never call the + // wrapper, so nothing else can retire the slot storage. + task->slot_retired.store(true); + ReleaseRef(task); // schedule: cancelled before dispatch + } else if (!task->periodic) { + // One-shot already dispatched: the wrapper may be between its dispatch + // (which made bthread_timer_del return 1) and its slot CAS, so it + // still has to touch the storage before it returns. Wait it out here, + // while this caller still owns the task through the owner reference, + // so that returning 0 really means "the storage is not used anymore". + // The wrapper retires the slot before calling any user code and takes + // no lock doing so, hence this wait cannot deadlock, and it is bounded + // by the OS scheduling of the timer thread. A periodic task needs no + // such wait: its slot storage is the pooled UbrTrx, which the callers + // of the non-blocking delete never free (only UbrMgrFini does, after + // UbrTimerDelAndWait), and its retire-slot load happens after the user + // callback -- which may take the very lock this caller holds. + while (!task->slot_retired.load()) { + bthread_usleep(kTimerPollIntervalUs); } - } - close(g_epoll_fd); - g_epoll_fd = -1; - g_total_timer_num = 0; - g_timer_module_initialized = 0; - int32_t ret = pthread_join(g_epoll_execute_thread, nullptr); - if (ret != EOK) { - LOG(ERROR) << "Failed to join pthread, during destroying timer module. ret=" << ret; - return; - } + } // ==1 periodic: dispatched/running; + // OnFire releases the schedule ref + ReleaseRef(task); // owner + return 0; // This call won the slot competition. For a one-shot timer, + // the callback will not run and the slot storage is unused. + // For a periodic timer, future rearming is stopped, but an + // already dispatched or running callback may still complete. } -RETURN_CODE TimerFdCtxValidate(uint32_t fd) { - if (fd >= g_max_system_fd) { - LOG(ERROR) << "TimerFd=" << fd << " is out of range=" << g_max_system_fd; - return UBRING_ERR; +void UbrTimerDelAndWait(butil::atomic* slot) { + if (slot == nullptr) { + return; } - if (g_timer_fd_ctx_map[fd].status == TIMER_CONTEXT_NOT_USING) { - LOG(ERROR) << "TimerFd=" << fd << " has wrong status=" << g_timer_fd_ctx_map[fd].status; - return UBRING_ERR; + // Called by the callback of that very timer: joining it would wait for + // ourselves, so degrade to the non-blocking delete (UbrTimerDel marks the + // task stopped and lets UbrTimerOnFire retire the slot on callback exit). + // Callers therefore do not need to know whether they run on the timer + // thread. + if (t_running_task != nullptr && slot->load() == t_running_task) { + UbrTimerDel(slot); + return; } - if (g_timer_fd_ctx_map[fd].cb == nullptr) { - LOG(ERROR) << "The callback is not set."; - return UBRING_ERR; + UbrTimerTask* task = TakeOutTask(slot); + if (task == nullptr) { + return; } - - return UBRING_OK; -} - -#if defined(OS_MACOSX) -static int timerfd_create_macosx(int clockid, int flags) { - int pipefd[2]; - if (pipe(pipefd) == -1) { - return -1; + task->join_pending.store(true); + task->stopped.store(true); + // A start still in flight cannot be cancelled yet; wait for the + // starter to schedule it or mark it dead, polling at + // kTimerPollIntervalUs. DelAndWait runs on teardown paths (force close, + // UbrMgrFini, connect failure) where a sub-millisecond delay is + // irrelevant, so polling keeps the facade free of a second wait primitive + // and works identically from bthreads and plain pthreads. + while (task->state.load() == kStarting) { + bthread_usleep(kTimerPollIntervalUs); + } + if (task->state.load() == kScheduled) { + bthread_timer_t id = task->id.load(); + if (id != 0 && bthread_timer_del(id) == 0) { + ReleaseRef(task); // cancelled before run + } } - return pipefd[0]; -} - -static int timerfd_settime_macosx(int fd, int flags, - const itimerspec *new_value, - itimerspec *old_value) { - if (old_value != nullptr) { - memset(old_value, 0, sizeof(itimerspec)); + ReleaseRef(task); // owner reference + while (!task->done.load()) { + bthread_usleep(kTimerPollIntervalUs); } - return 0; + task->join_pending.store(false); + delete task; } -#endif } // namespace ubring -} // namespace brpc \ No newline at end of file +} // namespace brpc diff --git a/src/brpc/ubshm/timer/timer_mgr.h b/src/brpc/ubshm/timer/timer_mgr.h index 1b42caef04..1c35a22499 100644 --- a/src/brpc/ubshm/timer/timer_mgr.h +++ b/src/brpc/ubshm/timer/timer_mgr.h @@ -15,59 +15,104 @@ // specific language governing permissions and limitations // under the License. +// bthread based timer facade for the ubring module. Callbacks run on the +// process-wide bthread timer thread and must return quickly. + #ifndef BRPC_TIMER_MGR_H #define BRPC_TIMER_MGR_H -#include -#include -#include "brpc/ubshm/common/common.h" - -#if defined(OS_LINUX) -#include -#include -#elif defined(OS_MACOSX) -#include -#include -#include -#endif -#define MAX_TIMER 1024 -#define TIMER_EPOLL_WAIT_TIMEOUT 1000 +#include +#include "brpc/ubshm/common/common.h" -#if defined(OS_MACOSX) -struct itimerspec -{ - struct timespec it_interval; - struct timespec it_value; -}; -#endif namespace brpc { namespace ubring { -typedef enum { - TIMER_CONTEXT_NOT_USING, - TIMER_CONTEXT_EPOLL_WAITING, - TIMER_CONTEXT_CALLBACK_ONGOING -} TimerFdCtxStatus; -typedef struct { - void *(*cb)(void*); - void *args; - uint32_t fd; - TimerFdCtxStatus status; - uint32_t periodical; - pthread_spinlock_t spin_lock; -} TimerFdCtx; +// Opaque timer handle. nullptr means "not started" (or already deleted / +// fired for one-shot timers). +typedef struct UbrTimerTask* UbrTimerId; + +// Maps the current re-arm interval of a periodic timer to the next one. +// Runs on the timer thread only. +typedef uint64_t (*UbrTimerBackoffFn)(void* arg, uint64_t cur_interval_us); + +// Schedule a one-shot `cb(arg, gen)' to run after `delay_us'. The handle slot +// is released before the callback runs, so the callback may free the object +// that stores the slot; the task object itself is released automatically. +// `slot' must point to a real butil::atomic object, so start and +// delete can race on one RMW without reinterpreting plain storage as an +// atomic. `gen' is an opaque value carried by the task and passed back on the +// fire, so a callback armed for an object that was later released and reused +// can detect that it is stale. It is required: passing a wrong value silently +// disables the generation guard, so callers must pass the generation of the +// object `arg' points to. +RETURN_CODE UbrTimerStart(butil::atomic* slot, uint64_t delay_us, + void* (*cb)(void*, uint64_t), void* arg, uint64_t gen); + +// Schedule `cb(arg, gen)' after `delay_us' and re-arm it every `interval_us' +// until it is deleted. `interval_us' must be positive: a periodic timer without +// a period is a misconfiguration and is rejected here, instead of silently +// degrading to a one-shot (which is what the interval-based API used to do). +// Unlike a one-shot timer, a periodic one retires its handle slot only after +// the callback returned, so the callback must NOT free the object that stores +// the slot. `backoff' maps the interval the timer is currently using to the +// next one; returning 0 is ignored (the previous interval is kept) so a broken +// back-off cannot turn the global timer thread into a spin loop. +RETURN_CODE UbrTimerStartPeriodic(butil::atomic* slot, + uint64_t delay_us, uint64_t interval_us, + void* (*cb)(void*, uint64_t), void* arg, + uint64_t gen, + UbrTimerBackoffFn backoff = nullptr); + +// Non-blocking delete, safe to call from inside the timer callback itself. +// This function does not wait for an already running user callback and does +// not protect resources reachable from `arg` on its own. +// +// Returns 0 when this call wins the handle-slot competition. +// - For a one-shot timer, the callback will not run. This call additionally +// waits out a wrapper that the timer thread already dispatched: such a +// wrapper may have been preempted between its dispatch and the moment it +// competes for the slot, and it still has to touch the slot once (to lose +// that competition) before it can return. When this call returns 0, the +// facade is therefore completely done with the storage `slot` points to, +// and the caller may free the object that stores the slot -- which is what +// the delayed-clear path does with UbrCleanupCtl. The wait only spins on a +// task-internal flag: the wrapper takes no lock and calls no user code +// while retiring the slot, so a caller holding any ubring lock can still +// wait, and no timer callback can be joined from its own dispatch. +// - For a periodic timer, future rearming is stopped, but an already +// dispatched or running callback may still execute once more, and the task +// stays anchored in the slot until that callback returns. Callers must not +// reclaim resources reachable from `arg` based on this return alone; use +// UbrTimerDelAndWait when teardown needs to wait for callbacks. A return of +// 0 therefore never means "the handle is free again". +// A periodic callback that deletes its own timer falls into the periodic case +// as well, but only marks it stopped: the handle slot stays anchored until the +// callback returns, so a concurrent UbrTimerDelAndWait on the same slot really +// does wait for that callback. +// +// Returns 1 when this caller did not acquire timer ownership. The callback, +// another deleter, or the scheduling path is responsible for settling the +// timer resources, so this caller must not reclaim them. +int UbrTimerDel(butil::atomic* slot); + +// Delete and wait until a possibly running callback finished, so the +// caller can free resources reachable from `arg'. Safe to call from inside the +// callback's own dispatch: it then degrades to the non-blocking UbrTimerDel +// instead of joining itself, so callers never have to know which thread they +// run on. +// +// A one-shot callback that is already running holds the ownership of `arg' by +// itself (mirroring bthread_timer_del returning 1) and cannot be waited for +// through the slot. Callers of one-shot timers must therefore keep `arg' alive +// by their own reference (this is a hard requirement, not a convenience: the +// delayed-clear path relies on UbrCleanupCtl's reference count for exactly +// this reason). A periodic callback -- including one that deleted its own +// timer -- always can be waited for, because its task stays anchored until it +// returns. The wait polls with bthread_usleep, which degrades to ::usleep on +// plain pthread callers (e.g. process-exit paths). +void UbrTimerDelAndWait(butil::atomic* slot); -RETURN_CODE TimerInit(void); -void TimerModuleDestroy(void); -void *UnifiedCallback(void *args); -void *TimerEpoll(void *args); -int32_t TimerStart(const itimerspec *time, void *(*cb)(void *), void *args); -uint32_t GetActiveTimerNum(void); -void CloseTimerFd(int fd); +} // namespace ubring +} // namespace brpc -void DeleteTimerSafe(uint32_t fd); -void DeleteTimer(uint32_t fd); -RETURN_CODE TimerFdCtxValidate(uint32_t fd); -} -} -#endif //BRPC_TIMER_MGR_H \ No newline at end of file +#endif //BRPC_TIMER_MGR_H diff --git a/src/brpc/ubshm/ub_cleanup_worker.cpp b/src/brpc/ubshm/ub_cleanup_worker.cpp new file mode 100644 index 0000000000..c53e351f24 --- /dev/null +++ b/src/brpc/ubshm/ub_cleanup_worker.cpp @@ -0,0 +1,175 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include +#include +#include "butil/atomicops.h" +#include "butil/containers/mpsc_queue.h" +#include "butil/logging.h" +#include "butil/scoped_lock.h" +#include "bthread/bthread.h" +#include "brpc/ubshm/ub_cleanup_worker.h" +#include "brpc/ubshm/ub_ring_manager.h" + +namespace brpc { +namespace ubring { + +// Test hook, never set in production: when non-zero, the lazy worker start +// inside Post() skips the real bthread_start_background call and behaves as +// if it had returned this value, so a transient start failure (e.g. ENOMEM) +// and its recovery can be exercised deterministically. It is only consulted +// while the worker is not started; read under g_worker_mtx, written by tests +// before posting. +butil::atomic g_ub_test_worker_start_rc(0); + +namespace { + +// Cleanup latency is tolerated in seconds (it only delays releasing memory of +// an already dead link), so the idle worker polls instead of spinning and the +// module needs no extra wakeup primitive. +constexpr uint64_t kCleanupIdlePollIntervalUs = 100 * 1000; + +butil::atomic g_worker_started(false); +// Posted-but-not-finished jobs. A poster increments it before enqueueing and +// the worker decrements it after the job returned, so DrainAndWait can wait +// for quiescence without observing the queue itself. +butil::atomic g_cleanup_inflight(0); +// The queue is never destroyed. The cleanup worker bthread outlives every +// orderly teardown -- the bthread runtime itself is not destructed at exit +// (see the note in task_control.cpp), so the worker can still be inside +// Dequeue() while static destructors run. MPSCQueue's destructor drains the +// queue, which would make that destruction a *second* concurrent consumer +// of a single-consumer queue (_cur_dequeue_node is a plain pointer) and +// double-free nodes; a timer callback firing during the destructor phase +// would additionally enqueue into a destroyed queue. Leaking the queue +// follows the brpc convention for globals entangled with threads that +// never stop (g_task_control itself); the kernel reclaims it at exit. +butil::MPSCQueue* const g_cleanup_queue = + new butil::MPSCQueue(); +// Serializes the posters (never taken by the worker) around the lazy start and +// the enqueue; see Post(). +pthread_mutex_t g_worker_mtx = PTHREAD_MUTEX_INITIALIZER; + +void RunTrxCleanup(const UbrCleanupJob& job) { + UbrCleanupCtl* ctl = job.ctl; + // The slot may have been released and reused while the job was queued; the + // cleanup body must then be skipped, but the control object still has to + // be settled (exactly as the timer callback did before the offload). + if (UBRingManager::IsUbrTrxSlotUsed(job.trx->trx_mgr_index, job.ubr_id)) { + job.work(job.trx, job.ubr_id); + } + ATOMIC_STORE(ctl->state, UBR_CLEANUP_DONE); + ctl->ReleaseRef(); // timer/callback reference +} + +void* UbrCleanupWorkerMain(void*) { + UbrCleanupJob job; + for (;;) { + if (!g_cleanup_queue->Dequeue(job)) { + bthread_usleep(kCleanupIdlePollIntervalUs); + continue; + } + if (job.type == UbrCleanupJob::SHM_DRAIN) { + job.shm_step(job.shm_list); + } else { + RunTrxCleanup(job); + } + g_cleanup_inflight.fetch_sub(1); + } + return nullptr; +} + +bool Post(const UbrCleanupJob& job) { + // Serialize the posters (the worker itself never takes this mutex) so + // that at most one cleanup worker is ever started: two posters seeing + // "not started" at the same time would start two bthreads -- two + // concurrent consumers of a single-consumer queue. The start attempt and + // the enqueue also share one critical section, so a job is only ever + // queued behind a worker whose start succeeded. Posting only happens on + // cleanup paths, so the mutex is free in practice. + BAIDU_SCOPED_LOCK(g_worker_mtx); + if (!g_worker_started.load()) { + // Retry the lazy start on every post instead of latching a failure: + // bthread_start_background can fail transiently (ENOMEM), and a + // permanent fallback would run every later SDK cleanup inline on the + // global timer thread -- exactly what the worker exists to avoid, + // and worst under the memory pressure that caused the failure. A + // failed attempt leaves g_worker_started false and fails only THIS + // post; the caller runs it inline as the safety net, and the next + // post retries the start. + bthread_attr_t attr = BTHREAD_ATTR_NORMAL; + bthread_attr_set_name(&attr, "UBCleanup"); + bthread_t tid; + // A non-zero test hook overrides the start result without creating + // a bthread; see g_ub_test_worker_start_rc. + const int injected_rc = g_ub_test_worker_start_rc.load( + butil::memory_order_relaxed); + const int rc = injected_rc != 0 ? injected_rc + : bthread_start_background(&tid, &attr, + UbrCleanupWorkerMain, nullptr); + if (BAIDU_UNLIKELY(rc != 0)) { + LOG_EVERY_SECOND(ERROR) << "Fail to start the ubring cleanup " + << "worker bthread, running the cleanup " + << "inline."; + return false; + } + g_worker_started.store(true); + } + // Increment before enqueueing so a concurrent DrainAndWait can never + // observe quiescence while this job is still on its way to the queue. + g_cleanup_inflight.fetch_add(1); + g_cleanup_queue->Enqueue(job); + return true; +} + +} // namespace + +bool UbrCleanupWorker::PostShmDrain(ShmList* shm_list, + void (*shm_step)(ShmList* shm_list)) { + if (BAIDU_UNLIKELY(shm_list == nullptr || shm_step == nullptr)) { + return false; + } + UbrCleanupJob job; + job.type = UbrCleanupJob::SHM_DRAIN; + job.shm_list = shm_list; + job.shm_step = shm_step; + return Post(job); +} + +bool UbrCleanupWorker::PostTrxCleanup(UbrTrx* trx, uint64_t ubr_id, + void (*work)(UbrTrx*, uint64_t), + UbrCleanupCtl* ctl) { + if (BAIDU_UNLIKELY(trx == nullptr || work == nullptr || ctl == nullptr)) { + return false; + } + UbrCleanupJob job; + job.type = UbrCleanupJob::TRX_CLEANUP; + job.trx = trx; + job.ubr_id = ubr_id; + job.work = work; + job.ctl = ctl; + return Post(job); +} + +void UbrCleanupWorker::DrainAndWait() { + while (g_cleanup_inflight.load() > 0) { + bthread_usleep(1000); + } +} + +} // namespace ubring +} // namespace brpc diff --git a/src/brpc/ubshm/ub_cleanup_worker.h b/src/brpc/ubshm/ub_cleanup_worker.h new file mode 100644 index 0000000000..93e90863ad --- /dev/null +++ b/src/brpc/ubshm/ub_cleanup_worker.h @@ -0,0 +1,96 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#ifndef BRPC_UB_CLEANUP_WORKER_H +#define BRPC_UB_CLEANUP_WORKER_H + +#include +#include "brpc/ubshm/common/common.h" +#include "brpc/ubshm/shm/shm_def.h" + +namespace brpc { +namespace ubring { + +struct TagUbrTrx; +typedef struct TagUbrTrx UbrTrx; +struct UbrCleanupCtl; + +// One unit of ubring cleanup work. +// +// The process-wide bthread timer thread runs every timer in the process (RPC +// timeouts, backup requests, bthread_usleep wakeups, ubring heartbeats and +// close checks), so a callback that calls into the UBS SDK -- which may wait +// on a network round trip to the SDK daemon -- delays all of them. The timer +// callbacks therefore only arbitrate ownership and post a job here; the +// blocking part runs on the dedicated cleanup worker. +struct UbrCleanupJob { + enum Type { + SHM_DRAIN = 0, // release one pending shared-memory unmap + TRX_CLEANUP = 1 // run the delayed cleanup of one trx generation + }; + + Type type = SHM_DRAIN; + + // SHM_DRAIN: `shm_step' performs the blocking SDK calls and settles the + // producer's retry bookkeeping. It is a function pointer so this worker + // stays independent of the shm backend, and so its scheduling can be + // unit-tested without the SDK. + void (*shm_step)(ShmList* shm_list) = nullptr; + ShmList* shm_list = nullptr; + + // TRX_CLEANUP: the job takes over the timer/callback reference the + // scheduling callback held on `ctl' and releases it when done. `work' is + // the cleanup body (used by UbrDoAsynClearWork / UbrDoPassiveClearWork) + // and only runs if the slot still belongs to `ubr_id'. + UbrTrx* trx = nullptr; + uint64_t ubr_id = 0; + void (*work)(UbrTrx* trx, uint64_t ubr_id) = nullptr; + UbrCleanupCtl* ctl = nullptr; +}; + +// A lazily started, process-wide bthread that drains UbrCleanupJob's. It is +// never stopped during a normal run: after a ShmMgrFini cycle the worker only +// needs to be quiescent (DrainAndWait) before the shared memory it may touch +// is finalized, and an idle worker performs no SDK call at all. Posting is +// serialized, so a job can never be queued behind a worker start that failed. +// A failed start (bthread creation may fail transiently, e.g. ENOMEM) is +// retried on the next post; only the posts made while the start keeps failing +// run inline on the caller. +class UbrCleanupWorker { +public: + // Post a shared-memory drain step. Returns false when the worker could not + // be started for this post (the start is retried on the next post), in + // which case the caller must run `shm_step' inline. + static bool PostShmDrain(ShmList* shm_list, void (*shm_step)(ShmList* shm_list)); + + // Post a delayed trx cleanup, transferring the timer/callback reference of + // `ctl'. Returns false when the worker could not be started for this post + // (the start is retried on the next post), in which case the caller keeps + // the reference and must run the cleanup inline. + static bool PostTrxCleanup(UbrTrx* trx, uint64_t ubr_id, + void (*work)(UbrTrx* trx, uint64_t ubr_id), + UbrCleanupCtl* ctl); + + // Wait until every posted job finished. Bounded by one in-flight SDK call; + // returns immediately when the worker never started. + static void DrainAndWait(); +}; + +} // namespace ubring +} // namespace brpc + +#endif //BRPC_UB_CLEANUP_WORKER_H diff --git a/src/brpc/ubshm/ub_endpoint.cpp b/src/brpc/ubshm/ub_endpoint.cpp index ec7e084217..e2fd99330f 100644 --- a/src/brpc/ubshm/ub_endpoint.cpp +++ b/src/brpc/ubshm/ub_endpoint.cpp @@ -614,7 +614,7 @@ void* UBShmEndpoint::ProcessHandshakeAtServer(void* arg) { } int result = snprintf(local_trx_shm.name, SHM_MAX_NAME_BUFF_LEN, "%s_%s", client_name, SERVER_SHM_NAME_SUFFIX); - if (UNLIKELY(result < 0)) { + if (BAIDU_UNLIKELY(result < 0)) { LOG(WARNING) << "Copy client shared memory name failed, ret=" << result; ub_transport->_ub_state = UBShmTransport::UB_OFF; } @@ -768,14 +768,14 @@ ssize_t UBShmEndpoint::CutFromIOBufList(butil::IOBuf** from, size_t ndata) { ssize_t nw = 0; errno = 0; nw = _ub_ring->UbrTrxWritev(vec, nvec); - if (UNLIKELY(nw == -1)) { + if (BAIDU_UNLIKELY(nw == -1)) { if (errno == EMSGSIZE) { LOG(ERROR) << "Non-blocking send msg failed, message is larger than ubring capacity."; } else { LOG(ERROR) << "Non-blocking send msg in failed, connection has been closed."; errno = EPIPE; } - } else if (UNLIKELY(nw == UBRING_RETRY)) { + } else if (BAIDU_UNLIKELY(nw == UBRING_RETRY)) { errno = EAGAIN; nw = -1; } @@ -957,7 +957,7 @@ int UBShmEndpoint::PollingModeInitialize(bthread_tag_t tag, } struct FnArgs { Poller* poller; - std::atomic* running; + butil::atomic* running; }; auto fn = [](void* p) -> void* { std::unique_ptr args(static_cast(p)); @@ -972,6 +972,7 @@ int UBShmEndpoint::PollingModeInitialize(bthread_tag_t tag, while (running->load(std::memory_order_relaxed)) { while (poller->op_queue.Dequeue(op)) { if (op.type == PollerSidOp::ADD) { + poller_sids.erase(op); poller_sids.emplace(op); } else if (op.type == PollerSidOp::REMOVE) { poller_sids.erase(op); diff --git a/src/brpc/ubshm/ub_endpoint.h b/src/brpc/ubshm/ub_endpoint.h index 526966c9f7..0d818ac562 100644 --- a/src/brpc/ubshm/ub_endpoint.h +++ b/src/brpc/ubshm/ub_endpoint.h @@ -257,7 +257,7 @@ friend class Socket; struct BAIDU_CACHELINE_ALIGNMENT PollerGroup { PollerGroup() : pollers(FLAGS_ub_poller_num), running(false) {} std::vector pollers; - std::atomic running; + butil::atomic running; }; static std::vector _poller_groups; diff --git a/src/brpc/ubshm/ub_helper.cpp b/src/brpc/ubshm/ub_helper.cpp index 230ca71bac..d3c769140d 100644 --- a/src/brpc/ubshm/ub_helper.cpp +++ b/src/brpc/ubshm/ub_helper.cpp @@ -59,10 +59,7 @@ static void GlobalUBInitializeOrDieImpl() { ExitWithError(); } - if (TimerInit()) { - PLOG(ERROR) << "Fail to TimerInit"; - ExitWithError(); - } + // Timers are bthread based and start lazily on first use. if (ShmMgrInit()) { PLOG(ERROR) << "Fail to ShmMgrInit"; diff --git a/src/brpc/ubshm/ub_ring.cpp b/src/brpc/ubshm/ub_ring.cpp index 595b96d03b..4b74b2575f 100644 --- a/src/brpc/ubshm/ub_ring.cpp +++ b/src/brpc/ubshm/ub_ring.cpp @@ -20,9 +20,11 @@ #include #include #include +#include #include "bthread/bthread.h" #include "butil/logging.h" #include "brpc/ubshm/ub_ring.h" +#include "brpc/ubshm/ub_cleanup_worker.h" #include "brpc/ubshm/ub_ring_manager.h" #include "brpc/ubshm/shm/shm_ipc.h" #include "brpc/ubshm/ubr_msg_v2.h" @@ -40,6 +42,14 @@ DEFINE_int32(ub_hb_retry_cnt, 10, "UBRing heartbeat retry count."); DEFINE_int32(ub_event_queue_timer_interval_us, 100, "UBRing disconnection check interval in microseconds."); +DEFINE_int32(ub_event_queue_timer_interval_max_us, 10000, + "UBRing upper bound of the close-check polling interval in " + "microseconds while the link is idle; the interval backs off " + "from ub_event_queue_timer_interval_us up to this value. " + "Set to 0 to keep the interval steady (back-off disabled)."); + +// Exponential back-off multiplier of the close-check polling interval. +constexpr uint64_t kCloseCheckBackoffFactor = 2; UBRing::UBRing() {} @@ -49,21 +59,164 @@ UBRing::~UBRing() RETURN_CODE UBRing::UbrTrxMapShm(SHM *local_shm, SHM *remote_shm) { RETURN_CODE rc = UbrTrxMapLocalShm(local_shm); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Trx map local shared memory failed."; return rc; } rc = UbrTrxMapRemoteShm(remote_shm); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Trx map remote shared memory failed."; return rc; } return UBRING_OK; } +// Stop a per-trx timer before its slot becomes reusable. UbrTimerDelAndWait +// returns only after a callback that was already dispatched has left the trx, +// which is what lets the caller clear the trx and free its shared memory +// afterwards. When the caller *is* the callback of that very timer, the facade +// recognizes it and degrades to the non-blocking delete (waiting would join the +// calling callback), so no caller has to know whether it runs on the timer +// thread. A sibling timer callback cannot be running concurrently either, +// because bthread dispatches every timer callback from one global timer thread +// (TimerThread in bthread/timer_thread.{h,cpp} is created by a single +// pthread_once). +static void UbrStopTrxTimer(butil::atomic* slot) { + UbrTimerDelAndWait(slot); +} + +static void UbrDoAsynClearWork(UbrTrx *trx, uint64_t expect_ubr_id) { + if (BAIDU_UNLIKELY(UBRing::UbrTrxFreeShm(trx) != UBRING_OK)) { + LOG(ERROR) << "Trx close, wait for local shm " << trx->local_shm.name << " free fail."; + } + if (BAIDU_UNLIKELY(UBRingManager::ReleaseUbrTrxFromMgr(trx, expect_ubr_id) != UBRING_OK)) { + LOG(ERROR) << "Trx close, release shm " << trx->local_shm.name << " trx failed."; + } +} + +static void UbrDoPassiveClearWork(UbrTrx *trx, uint64_t expect_ubr_id) { + int rc = ShmLocalFree(&trx->remote_shm); + if (rc != UBRING_OK) { + LOG(ERROR) << "Trx passive clear, delete remote shm " << trx->remote_shm.name + << " failed. ret=" << rc; + } + rc = ShmLocalFree(&trx->local_shm); + if (rc != UBRING_OK) { + LOG(ERROR) << "Trx passive clear, delete local shm " << trx->local_shm.name + << " failed. ret=" << rc; + } + if (BAIDU_UNLIKELY(UBRingManager::ReleaseUbrTrxFromMgr(trx, expect_ubr_id) != UBRING_OK)) { + LOG(ERROR) << "Trx passive clear, release shm " << trx->local_shm.name << " trx failed."; + } +} + +// Schedule the delayed cleanup of `trx'. The cleanup ownership lives in the +// per-acquisition control object, so exactly one of the delayed-clear +// callback and a force close ever runs the cleanup. `work' is the cleanup +// body, used directly when the timer cannot be started. +static RETURN_CODE UbrScheduleClearTimer(UbrTrx *trx, uint64_t expect_ubr_id, + void* (*cb)(void*, uint64_t), + void (*work)(UbrTrx*, uint64_t)) { + if (BAIDU_UNLIKELY(trx == nullptr || trx->local_shm.addr == nullptr)) { + return UBRING_OK; // released trx, stale event + } + // A callback that outlived its generation must not capture the id of the + // slot's new occupant, nor schedule cleanup for that new transaction. + if (BAIDU_UNLIKELY(ATOMIC_LOAD(trx->ubr_id) != expect_ubr_id)) { + return UBRING_OK; // stale event on a reused slot + } + if (trx->cleanup_ctl.load() != nullptr) { + return UBRING_OK; // cleanup already scheduled + } + auto* ctl = new (std::nothrow) UbrCleanupCtl(); + if (BAIDU_UNLIKELY(ctl == nullptr)) { + LOG(ERROR) << "Fail to malloc ubr cleanup ctl."; + return UBRING_ERR; + } + ctl->trx = trx; + ctl->ubr_id = expect_ubr_id; + ctl->state.store(UBR_CLEANUP_PENDING); + ctl->timer = nullptr; + ctl->ref.store(2); // timer/callback + starter; the + // manager anchor is taken by + // TryPublishUnitCleanupCtl + + // The manager publishes the ctl on the trx and anchors it in the pool slot + // in one critical section, so a concurrent force close can never observe + // the trx-side publication without its anchor. + if (!UBRingManager::TryPublishUnitCleanupCtl(trx->trx_mgr_index, + ctl->ubr_id, ctl)) { + // Another schedule won the slot, or the slot was released (and + // possibly reused) before we could anchor: force close or the new + // occupant owns it now. Nothing was armed or published -- drop our + // two references. + ctl->ReleaseRef(); // timer/callback reference, never armed + ctl->ReleaseRef(); // starter reference + return UBRING_OK; + } + // One-shot on purpose: this only delays the cleanup so in-flight IO can + // drain, and FLAGS_ub_flying_io_timeout_s == 0 legitimately means "do not + // delay". + RETURN_CODE rc = UbrTimerStart(&ctl->timer, + (uint64_t)FLAGS_ub_flying_io_timeout_s * SEC_TO_USEC, cb, ctl, + expect_ubr_id); + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { + // The timer was never scheduled: this path owns the manager, + // timer/callback and starter references. Hand the cleanup to the + // worker -- the caller may be the timer thread -- so the trx does not + // end up with neither timers nor a queued cleanup, and run it inline + // only when the worker cannot be started. + int state_expected = UBR_CLEANUP_PENDING; + if (ATOMIC_COMPARE_EXCHANGE_STRONG(ctl->state, state_expected, UBR_CLEANUP_RUNNING)) { + if (!UbrCleanupWorker::PostTrxCleanup(trx, ctl->ubr_id, work, ctl)) { + // Gate on the slot still being used by this generation: force + // close may have claimed the cleanup and released the slot + // (which already freed the trx resources) after we were + // anchored. + if (UBRingManager::IsUbrTrxSlotUsed(trx->trx_mgr_index, ctl->ubr_id)) { + work(trx, ctl->ubr_id); + } + ATOMIC_STORE(ctl->state, UBR_CLEANUP_DONE); + ctl->ReleaseRef(); // timer/callback reference + } + // When the job was posted, the worker owns the timer/callback + // reference and settles state/anchor exactly like the timer-fired + // path; the manager anchor keeps the control object alive until + // the slot is acquired again or UbrMgrFini retires it. + } else { + // A force close already owns the cleanup. It could not take the + // reference through the never-armed timer, so release it here. + ctl->ReleaseRef(); // timer/callback reference + } + ctl->ReleaseRef(); // starter reference + return UBRING_ERR; + } + if (BAIDU_UNLIKELY(ATOMIC_LOAD(trx->ubr_id) != ctl->ubr_id)) { + // Published onto a slot that was released and reused meanwhile. + if (UbrTimerDel(&ctl->timer) == 0) { + ctl->ReleaseRef(); // timer/callback reference + } + UBRingManager::DetachUnitCleanupCtl(trx->trx_mgr_index, ctl); + ctl->ReleaseRef(); // starter reference + return UBRING_OK; + } + ctl->ReleaseRef(); // starter reference + return UBRING_OK; +} + RETURN_CODE UBRing::UbrTrxClose() { - RETURN_CODE close_check_rc = UbrTrxCloseCheck(_trx); - if (UNLIKELY(close_check_rc != UBRING_OK)) { + // _trx is still nullptr when the setup failed: UbrAllocateLocalShm and + // UbrAllocateServerShm reset it on their error paths while the endpoint + // keeps the non-null _ub_ring and calls UbrTrxClose from + // DeallocateResources. Reject before the generation load, which would + // dereference the null _trx. + if (BAIDU_UNLIKELY(_trx == nullptr)) { + LOG(ERROR) << "Trx close failed, client trx is null."; + return UBRING_ERR; + } + const uint64_t expect_ubr_id = ATOMIC_LOAD(_trx->ubr_id); + RETURN_CODE close_check_rc = UbrTrxCloseCheck(_trx, expect_ubr_id); + if (BAIDU_UNLIKELY(close_check_rc != UBRING_OK)) { if (close_check_rc == UBRING_REENTRY) { LOG(INFO) << "Trx close skipped, already closing, local name=" << _trx->local_shm.name; return UBRING_OK; @@ -91,17 +244,48 @@ RETURN_CODE UBRing::UbrTrxClose() { LOG(WARNING) << "Local shm " << _trx->local_shm.name << " wait for the peer to close timed out, force cleanup."; _trx->ubr_rx.trx_state = UBR_STATE_CLOSED; - // Force synchronous cleanup instead of relying on async timer - DeleteTimerSafe((uint32_t)_trx->timer_fd); - DeleteTimerSafe((uint32_t)_trx->hb_timer_fd); - if (_trx->ubr_tx.remote_rx_event_q.addr != nullptr) { - ((UbrEventQMsg *)_trx->ubr_tx.remote_rx_event_q.addr)->flag = UBR_STATE_CLOSED; + // Wait out the close/heartbeat callbacks, which may schedule a + // delayed cleanup, then settle the cleanup ownership: force + // runs the cleanup itself when it can claim it, and leaves it + // to an already running delayed-clear callback otherwise. + UbrTimerDelAndWait(&_trx->close_timer); + UbrTimerDelAndWait(&_trx->hb_timer); + // Snapshot the delayed cleanup and, when there is none, claim the + // cleanup for this force close in one critical section. Splitting + // the two would let a concurrent SDK-fault callback publish a new + // cleanup after our empty snapshot, leaving two paths running the + // cleanup of the same shared memory. + UbrCleanupCtl* ctl = nullptr; + const UbrCleanupClaim claim = UBRingManager::ClaimTrxCleanupForced( + _trx->trx_mgr_index, expect_ubr_id, &ctl); + bool cleanup_owned = false; + if (claim == UBR_CLEANUP_CLAIM_HAS_CTL) { + int expected = UBR_CLEANUP_PENDING; + if (ATOMIC_COMPARE_EXCHANGE_STRONG(ctl->state, expected, UBR_CLEANUP_RUNNING)) { + cleanup_owned = true; + if (UbrTimerDel(&ctl->timer) == 0) { + ctl->ReleaseRef(); // timer/callback reference + } + } + } else if (claim == UBR_CLEANUP_CLAIM_OWNED_NULL) { + cleanup_owned = true; } - if (UNLIKELY(UbrTrxFreeShm(_trx) != UBRING_OK)) { - LOG(WARNING) << "Force close, local shm " << _trx->local_shm.name << " free failed."; + if (cleanup_owned) { + if (_trx->ubr_tx.remote_rx_event_q.addr != nullptr) { + ((UbrEventQMsg *)_trx->ubr_tx.remote_rx_event_q.addr)->flag = UBR_STATE_CLOSED; + } + if (BAIDU_UNLIKELY(UbrTrxFreeShm(_trx) != UBRING_OK)) { + LOG(WARNING) << "Force close, local shm " << _trx->local_shm.name << " free failed."; + } + if (BAIDU_UNLIKELY(UBRingManager::ReleaseUbrTrxFromMgr(_trx, expect_ubr_id) != UBRING_OK)) { + LOG(WARNING) << "Force close, release trx " << _trx->local_shm.name << " failed."; + } + if (ctl != nullptr) { + ATOMIC_STORE(ctl->state, UBR_CLEANUP_DONE); + } } - if (UNLIKELY(UBRingManager::ReleaseUbrTrxFromMgr(_trx) != UBRING_OK)) { - LOG(WARNING) << "Force close, release trx " << _trx->local_shm.name << " failed."; + if (ctl != nullptr) { + ctl->ReleaseRef(); // snapshot reference } return UBRING_ERR_TIMEOUT; } @@ -109,7 +293,7 @@ RETURN_CODE UBRing::UbrTrxClose() { } _trx->ubr_rx.trx_state = UBR_STATE_CLOSED; RETURN_CODE rc; - if (UNLIKELY((rc = ClearTrxResource(_trx, start_time, UBR_SEND_CLOSE)) != UBRING_OK)) { + if (BAIDU_UNLIKELY((rc = ClearTrxResource(_trx, expect_ubr_id)) != UBRING_OK)) { if (rc == UBRING_REENTRY) { LOG(INFO) << "Trx close, peer is closing, trx local name=" << _trx->local_shm.name; return UBRING_OK; @@ -126,62 +310,171 @@ RETURN_CODE UBRing::UbrTrxClose() { return UBRING_OK; } +// Back-off policy of the close-check timer: fast while there is traffic or +// a close in progress, doubling up to the cap while idle. +static uint64_t UbrCloseTimerBackoff(void* arg, uint64_t cur_interval_us) { + auto* trx = (UbrTrx*)arg; + auto* local_rx_event_q = (UbrEventQMsg *)trx->ubr_rx.local_rx_event_q.addr; + auto* local_tx_event_q = (UbrEventQMsg *)trx->ubr_tx.local_tx_event_q.addr; + const uint64_t in_io_id = ATOMIC_LOAD(trx->ubr_rx.in_io_id); + const uint64_t out_io_id = ATOMIC_LOAD(trx->ubr_tx.out_io_id); + if (BAIDU_UNLIKELY(local_rx_event_q == nullptr)) { + return (uint64_t)FLAGS_ub_event_queue_timer_interval_us; + } + const bool has_traffic = (in_io_id != trx->close_chk_in_io_id) || + (out_io_id != trx->close_chk_out_io_id); + const bool closing = (local_rx_event_q->flag != UBR_STATE_CONNECTED); + trx->close_chk_in_io_id = in_io_id; + trx->close_chk_out_io_id = out_io_id; + if (has_traffic || closing || local_tx_event_q == nullptr) { + return (uint64_t)FLAGS_ub_event_queue_timer_interval_us; + } + uint64_t next = cur_interval_us * kCloseCheckBackoffFactor; + const uint64_t max_us = (uint64_t)FLAGS_ub_event_queue_timer_interval_max_us; + if (max_us > 0 && next > max_us) { + next = max_us; + } else if (max_us == 0) { + next = cur_interval_us; + } + return next; +} + RETURN_CODE UBRing::UbrAddCloseTimer() { - if (UNLIKELY(_trx == nullptr)) { + if (BAIDU_UNLIKELY(_trx == nullptr)) { LOG(ERROR) << "Trx add close timer failed, trx is null."; return UBRING_ERR; } - const uint32_t event_q_timer_interval_ns = - FLAGS_ub_event_queue_timer_interval_us * USEC_TO_NSEC; - itimerspec time_spec = { - .it_interval = {.tv_sec = 0, .tv_nsec = event_q_timer_interval_ns}, - .it_value = {.tv_sec = 0, .tv_nsec = 1} - }; - int timer_fd = TimerStart(&time_spec, UbrTrxCloseCallback, (void*)_trx); - if (UNLIKELY(timer_fd == -1)) { + // Name the offending flag: the facade would also reject a zero interval, + // but only this log can point at the configuration that produced it. + if (BAIDU_UNLIKELY(FLAGS_ub_event_queue_timer_interval_us <= 0)) { + LOG(ERROR) << "Start ubr close timer failed, ub_event_queue_timer_interval_us=" + << FLAGS_ub_event_queue_timer_interval_us << " must be positive."; + return UBRING_ERR; + } + const uint32_t interval_us = FLAGS_ub_event_queue_timer_interval_us; + _trx->close_chk_in_io_id = ATOMIC_LOAD(_trx->ubr_rx.in_io_id); + _trx->close_chk_out_io_id = ATOMIC_LOAD(_trx->ubr_tx.out_io_id); + RETURN_CODE rc = UbrTimerStartPeriodic(&_trx->close_timer, 0, interval_us, + UbrTrxCloseCallback, (void*)_trx, + ATOMIC_LOAD(_trx->ubr_id), + UbrCloseTimerBackoff); + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Start ubr close timer failed, trx local name=" << _trx->local_shm.name; return UBRING_ERR; } - _trx->timer_fd = timer_fd; return UBRING_OK; } RETURN_CODE UBRing::UbrAddTimer() { - if (UNLIKELY(UbrAddCloseTimer() != UBRING_OK)) { - LOG(ERROR) << "Ubr " << _trx->local_shm.name << " add closed timer failed."; + // The failure branch below reports _trx->local_shm.name, so a null _trx + // would crash before the guards in UbrAddCloseTimer/UbrAddHBTimer get to + // return their error. No current caller reaches this with a null _trx; + // reject it anyway for the same reason those two do. + if (BAIDU_UNLIKELY(_trx == nullptr)) { + LOG(ERROR) << "Trx add timer failed, trx is null."; return UBRING_ERR; } - - if (UNLIKELY(UbrAddHBTimer() != UBRING_OK)) { - DeleteTimerSafe((uint32_t)_trx->timer_fd); - LOG(ERROR) << "Ubr " << _trx->local_shm.name << " add heartbeat timer failed."; + // Arm both timers with the manager lock held and only while the manager is + // not shutting down: UbrMgrFini then knows that no per-trx timer can appear + // after it took the shutdown flag, so waiting once for the timers it + // snapshotted is enough. Nothing here may block on a timer callback -- the + // callbacks take the manager lock themselves. + bool armed_close = false; + bool armed_hb = false; + const bool allowed = UBRingManager::ArmTimersExclusive( + [this, &armed_close, &armed_hb]() { + armed_close = (UbrAddCloseTimer() == UBRING_OK); + if (armed_close) { + armed_hb = (UbrAddHBTimer() == UBRING_OK); + } + return armed_close && armed_hb; + }); + if (BAIDU_UNLIKELY(!allowed || !armed_close || !armed_hb)) { + // Disarm outside the arming exclusion: waiting for a dispatched + // callback while holding the manager lock would deadlock against the + // callbacks that take it. + UbrTimerDelAndWait(&_trx->close_timer); + UbrTimerDelAndWait(&_trx->hb_timer); + if (!allowed) { + // A shutdown refusal is expected, not a failure to report as one. + LOG(WARNING) << "Ubr " << _trx->local_shm.name + << " timer not armed, ubr manager is shutting down."; + } else { + LOG(ERROR) << "Ubr " << _trx->local_shm.name << " add timer failed, close=" + << armed_close << ", hb=" << armed_hb; + } return UBRING_ERR; } return UBRING_OK; } -void* UBRing::UbrTrxCloseCallback(void* args) { +// Per-trx timer callbacks run on the process-wide bthread timer thread and can +// still be dispatched after the UBRing that armed them started tearing down. +// Every access to the pooled UbrTrx is therefore gated on the generation the +// timer was armed with (the `gen' argument): the callbacks reject a stale fire +// here, and every scheduling path re-checks expect_ubr_id under +// g_ubr_trx_mgr_mtx (TryPublishUnitCleanupCtl) and again before running the +// cleanup work (IsUbrTrxSlotUsed). +// +// A slot cannot be released and reused underneath a callback that already +// passed that check, and the guarantee is a synchronization one, not a timing +// one. bthread dispatches all timer callbacks from a single global timer +// thread, so two per-trx callbacks never run concurrently, and the delayed +// clear arbitration (the ctl->state compare-exchange in the clear callbacks) +// is serialized after every per-trx callback that was dispatched before it. +// The cleanup body itself no longer runs on the timer thread but on the +// cleanup worker (UbrCleanupWorker): it is kept away from a released or reused +// slot by the generation gate it re-applies under g_ubr_trx_mgr_mtx right +// before running the work (IsUbrTrxSlotUsed), and by the scheduling paths that +// wait for the per-trx callbacks through UbrStopTrxTimer -> UbrTimerDelAndWait +// before they may clear the trx or hand its slot on. Entering a close claims +// the slot for the generation under g_ubr_trx_mgr_mtx +// (UBRingManager::TryClaimTrxClose), so a caller working from a stale snapshot +// -- the faulty-shm event, which looks the slot up before the claim -- cannot +// touch the next occupant. Teardown that happens on any other thread then waits +// for a dispatched callback through UbrStopTrxTimer -> UbrTimerDelAndWait +// before it clears the trx or frees its shared memory (UbrClearResourceCheck, +// UbrPassiveClearTrx), and the force close and UbrMgrFini paths do the same. +// That wait also covers a callback that stopped its own periodic timer from +// inside: the timer facade keeps such a task anchored until the callback +// returns precisely so these teardown paths cannot mistake a running callback +// for an idle trx. FLAGS_ub_flying_io_timeout_s only delays the delayed-clear +// work so that in-flight IO can drain; the callback side is already ordered by +// the timer thread. The UbrTrx pool itself is freed only by UbrMgrFini, which +// waits for these timers first. +void* UBRing::UbrTrxCloseCallback(void* args, uint64_t gen) { auto* trx = (UbrTrx*) args; - if (UNLIKELY(UBRing::UbrTrxCallbackCheck(trx) != UBRING_OK)) { + // UbrTrxCallbackCheck rejects a null trx (and cleared queues) before we + // dereference trx->ubr_id for the stale-generation check below. + if (BAIDU_UNLIKELY(UBRing::UbrTrxCallbackCheck(trx) != UBRING_OK)) { + return nullptr; + } + // Reject a stale fire whose trx slot was released and reused: it must not + // touch the slot's new occupant. + if (BAIDU_UNLIKELY(ATOMIC_LOAD(trx->ubr_id) != gen)) { return nullptr; } auto* local_rx_event_q = (UbrEventQMsg *)trx->ubr_rx.local_rx_event_q.addr; auto* local_tx_event_q = (UbrEventQMsg *)trx->ubr_tx.local_tx_event_q.addr; + // UbrTrxCallbackCheck validated these before the generation check; repeat + // it here because a concurrent release may have cleared them since, and the + // dereference below is unconditional. UbrTrxHBCallback carries the same + // guard. + if (BAIDU_UNLIKELY(local_rx_event_q == nullptr || local_tx_event_q == nullptr)) { + return nullptr; + } if (local_rx_event_q->flag != UBR_STATE_CLOSED || local_tx_event_q->flag == UBR_STATE_CLOSED) { return nullptr; } trx->ubr_rx.trx_state = UBR_STATE_CLOSED; - int fd = (int)trx->local_shm.fd; do { if (ATOMIC_LOAD(trx->close_cnt) == 0) { break; } ATOMIC_SUB(trx->close_cnt, 1); - uint64_t start_time = GetCurNanoSeconds(); - if (local_tx_event_q->flag == UBR_STATE_CONNECTED || ATOMIC_LOAD(trx->close_cnt) == 1) { local_tx_event_q->flag = UBR_STATE_CLOSED; trx->ubr_tx.trx_state = UBR_STATE_CLOSED; @@ -192,8 +485,8 @@ void* UBRing::UbrTrxCloseCallback(void* args) { break; } remote_rx_event_q->flag = UBR_STATE_CLOSED; - RETURN_CODE clear_rc = ClearTrxResource(trx, start_time, UBR_CALL_BACK_CLOSE, 1); - if (UNLIKELY(clear_rc != UBRING_OK && clear_rc != UBRING_REENTRY)) { + RETURN_CODE clear_rc = ClearTrxResource(trx, gen); + if (BAIDU_UNLIKELY(clear_rc != UBRING_OK && clear_rc != UBRING_REENTRY)) { LOG(ERROR) << "Trx close callback failed, " << trx->local_shm.name << " clear trx resource failed."; break; } @@ -202,70 +495,99 @@ void* UBRing::UbrTrxCloseCallback(void* args) { } RETURN_CODE UBRing::UbrAddHBTimer() { - if (UNLIKELY(_trx == nullptr)) { + if (BAIDU_UNLIKELY(_trx == nullptr)) { LOG(ERROR) << "Trx add heartbeat timer failed, trx is null."; return UBRING_ERR; } - itimerspec time_spec = { - .it_interval = {.tv_sec = FLAGS_ub_hb_timer_interval_s, .tv_nsec = 0}, - .it_value = {.tv_sec = 0, .tv_nsec = 1} - }; - int timer_fd = TimerStart(&time_spec, UbrTrxHBCallback, (void*)_trx); - if (UNLIKELY(timer_fd == -1)) { + // A zero or negative ub_hb_timer_interval_s would make the heartbeat fire + // once (or, after the unsigned conversion, effectively never), so the link + // would silently lose its liveness detection. UbrTimerStartPeriodic + // rejects the zero interval; reject the negative one here as well. + if (BAIDU_UNLIKELY(FLAGS_ub_hb_timer_interval_s <= 0)) { + LOG(ERROR) << "Start ubr heartbeat timer failed, ub_hb_timer_interval_s=" + << FLAGS_ub_hb_timer_interval_s << " must be positive."; + return UBRING_ERR; + } + const uint64_t interval_us = (uint64_t)FLAGS_ub_hb_timer_interval_s * SEC_TO_USEC; + RETURN_CODE rc = UbrTimerStartPeriodic(&_trx->hb_timer, 0, interval_us, + UbrTrxHBCallback, (void*)_trx, + ATOMIC_LOAD(_trx->ubr_id)); + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Start ubr heartbeat timer failed."; return UBRING_ERR; } - _trx->hb_timer_fd = timer_fd; return UBRING_OK; } -RETURN_CODE UBRing::UbrPassiveClearTrx(UbrTrx *trx, int fd, PASSIVE_DISC_TYPE type) { - RETURN_CODE passive_close_check_rc = UbrTrxCloseCheck(trx); - if (UNLIKELY(passive_close_check_rc != UBRING_OK)) { +RETURN_CODE UBRing::UbrPassiveClearTrx(UbrTrx *trx, uint64_t expect_ubr_id) { + RETURN_CODE passive_close_check_rc = UbrTrxCloseCheck(trx, expect_ubr_id); + if (BAIDU_UNLIKELY(passive_close_check_rc != UBRING_OK)) { if (passive_close_check_rc == UBRING_REENTRY) { LOG(INFO) << "Passive close skipped, active close in progress, name=" << trx->local_shm.name; - uint64_t start_time = GetCurNanoSeconds(); - return ClearTrxResource(trx, start_time, UBR_CALL_BACK_CLOSE); + return ClearTrxResource(trx, expect_ubr_id); } + // UBRING_ERR means this generation no longer owns the slot: the event + // belongs to a trx that was released (and possibly reused) meanwhile, + // so its state and timers must not be touched at all. return UBRING_ERR; } trx->ubr_tx.trx_state = UBR_STATE_CLOSED; trx->ubr_rx.trx_state = UBR_STATE_CLOSED; - DeleteTimerSafe((uint32_t)trx->timer_fd); - const char *type_name = nullptr; - if (type == UBR_HEARTBEAT) { - DeleteTimer((uint32_t)trx->hb_timer_fd); - type_name = "Trx heartbeat"; - } else if (type == UBR_UB_EVENT) { - DeleteTimerSafe((uint32_t)trx->hb_timer_fd); - type_name = "Ub event callback"; - } - constexpr int64_t kMicrosecondsPerSecond = 1000000LL; - bthread_usleep(FLAGS_ub_flying_io_timeout_s * kMicrosecondsPerSecond); + UbrStopTrxTimer(&trx->close_timer); + UbrStopTrxTimer(&trx->hb_timer); + // Wait for in-flight IO on a one-shot timer instead of sleeping on the + // timer thread. + return UbrScheduleClearTimer(trx, expect_ubr_id, + UbrPassiveClearCallback, UbrDoPassiveClearWork); +} - int rc = ShmLocalFree(&trx->remote_shm); - if (rc != UBRING_OK) { - LOG(ERROR) << type_name << ", delete remote shm failed. ret=" << rc; +void* UBRing::UbrPassiveClearCallback(void* args, uint64_t) { + auto* ctl = (UbrCleanupCtl*)args; + if (BAIDU_UNLIKELY(ctl == nullptr)) { + LOG(ERROR) << "Trx passive clear callback failed, ctl is null."; + return nullptr; } - rc = ShmLocalFree(&trx->local_shm); - if (rc != UBRING_OK) { - LOG(ERROR) << type_name << ", delete local shm failed. ret=" << rc; + int expected = UBR_CLEANUP_PENDING; + if (!ATOMIC_COMPARE_EXCHANGE_STRONG(ctl->state, + expected, UBR_CLEANUP_RUNNING)) { + // Force close owns the cleanup; this fire still holds the + // timer/callback reference inherited from the schedule. + ctl->ReleaseRef(); + return nullptr; } - - UBRingManager::ReleaseUbrTrxFromMgr(trx); - return UBRING_OK; + // The ownership arbitration above is settled; hand the blocking work to + // the cleanup worker instead of running it on the timer thread, which + // serves every timer in the process. The job takes over the timer/callback + // reference. + UbrTrx* trx = ctl->trx; + if (!UbrCleanupWorker::PostTrxCleanup(trx, ctl->ubr_id, + UbrDoPassiveClearWork, ctl)) { + if (BAIDU_UNLIKELY(UBRingManager::IsUbrTrxSlotUsed(trx->trx_mgr_index, ctl->ubr_id))) { + UbrDoPassiveClearWork(trx, ctl->ubr_id); + } + ATOMIC_STORE(ctl->state, UBR_CLEANUP_DONE); + ctl->ReleaseRef(); // timer/callback reference + } + return nullptr; } -void* UBRing::UbrTrxHBCallback(void* args) { +void* UBRing::UbrTrxHBCallback(void* args, uint64_t gen) { auto* trx = (UbrTrx*) args; - if (UNLIKELY(UbrTrxCallbackCheck(trx) != UBRING_OK)) { + // UbrTrxCallbackCheck rejects a null trx (and cleared queues) before we + // dereference trx->ubr_id for the stale-generation check below. + if (BAIDU_UNLIKELY(UbrTrxCallbackCheck(trx) != UBRING_OK)) { + return nullptr; + } + // Reject a stale fire whose trx slot was released and reused: it must not + // touch the slot's new occupant. + if (BAIDU_UNLIKELY(ATOMIC_LOAD(trx->ubr_id) != gen)) { return nullptr; } auto* local_data_status = (UbrDataStatusQMsg *)trx->ubr_tx.local_data_status_q.addr; auto* remote_data_status = (UbrDataStatusQMsg *)trx->ubr_rx.remote_data_status_q.addr; - if (UNLIKELY(local_data_status == nullptr || remote_data_status == nullptr)) { + if (BAIDU_UNLIKELY(local_data_status == nullptr || remote_data_status == nullptr)) { LOG(ERROR) << "Heartbeat error, datastatus is NULL."; return nullptr; } @@ -288,50 +610,46 @@ void* UBRing::UbrTrxHBCallback(void* args) { } int fd = (int)trx->local_shm.fd; - LOG(INFO) << "Hlc heartbeat, start to clear trx resource. hb_timer_fd=" << fd << ", shm_name=" << trx->local_shm.name; - UbrPassiveClearTrx(trx, fd, UBR_HEARTBEAT); - LOG(INFO) << "Hlc heartbeat clear trx resource finish."; + LOG(INFO) << "Ubr heartbeat, start to clear trx resource. shm_fd=" << fd << ", shm_name=" << trx->local_shm.name; + UbrPassiveClearTrx(trx, gen); + LOG(INFO) << "Ubr heartbeat clear trx resource finish."; return nullptr; } -RETURN_CODE UBRing::UbrAddAsynClearTimer(UbrTrx *trx) { - if (UNLIKELY(trx == nullptr)) { +RETURN_CODE UBRing::UbrAddAsynClearTimer(UbrTrx *trx, uint64_t expect_ubr_id) { + if (BAIDU_UNLIKELY(trx == nullptr)) { LOG(ERROR) << "Trx add close timer failed, trx is null."; return UBRING_ERR; } - - if (trx->clear_timer_fd > 0) { - return UBRING_OK; - } - - itimerspec time_spec = { - .it_interval = {.tv_sec = 0, .tv_nsec = 0}, - .it_value = {.tv_sec = FLAGS_ub_flying_io_timeout_s, .tv_nsec = 0} - }; - - int timer_fd = TimerStart(&time_spec, UbrAsynClearCallback, (void*)trx); - if (UNLIKELY(timer_fd == -1)) { - LOG(ERROR) << "Start ubr close timer failed, trx name=%s.", trx->local_shm.name; - return UBRING_ERR; - } - trx->clear_timer_fd = timer_fd; - return UBRING_OK; + return UbrScheduleClearTimer(trx, expect_ubr_id, + UbrAsynClearCallback, UbrDoAsynClearWork); } -void *UBRing::UbrAsynClearCallback(void *args) +void *UBRing::UbrAsynClearCallback(void *args, uint64_t) { - auto* trx = (UbrTrx*) args; - if (UNLIKELY(trx == nullptr)) { - LOG(ERROR) << "Trx close, trx is null."; + auto* ctl = (UbrCleanupCtl*) args; + if (BAIDU_UNLIKELY(ctl == nullptr)) { + LOG(ERROR) << "Trx close, ctl is null."; return nullptr; } - - if (UNLIKELY(UbrTrxFreeShm(trx) != UBRING_OK)) { - LOG(ERROR) << "Trx close, wait for local shm " << trx->local_shm.name << " free fail."; + int expected = UBR_CLEANUP_PENDING; + if (!ATOMIC_COMPARE_EXCHANGE_STRONG(ctl->state, + expected, UBR_CLEANUP_RUNNING)) { + // Force close owns the cleanup; this fire still holds the + // timer/callback reference inherited from the schedule. + ctl->ReleaseRef(); + return nullptr; } - - if (UNLIKELY(UBRingManager::ReleaseUbrTrxFromMgr(trx) != UBRING_OK)) { - LOG(ERROR) << "Trx close, release shm " << trx->local_shm.name << " trx failed."; + // Same offload as the passive path: the timer thread only arbitrates + // ownership, the worker does the SDK-backed cleanup. + UbrTrx* trx = ctl->trx; + if (!UbrCleanupWorker::PostTrxCleanup(trx, ctl->ubr_id, + UbrDoAsynClearWork, ctl)) { + if (BAIDU_UNLIKELY(UBRingManager::IsUbrTrxSlotUsed(trx->trx_mgr_index, ctl->ubr_id))) { + UbrDoAsynClearWork(trx, ctl->ubr_id); + } + ATOMIC_STORE(ctl->state, UBR_CLEANUP_DONE); + ctl->ReleaseRef(); // timer/callback reference } return nullptr; } @@ -342,7 +660,7 @@ int UBRing::UbrTrxSend(const void *buf, uint32_t buf_len) errno = EPROTONOSUPPORT; return UBRING_ERR; } - if (UNLIKELY(CheckTrxSendPreCheck(_trx) != UBRING_OK)) { + if (BAIDU_UNLIKELY(CheckTrxSendPreCheck(_trx) != UBRING_OK)) { return UBRING_ERR; } // 1.2 Calculate space @@ -366,8 +684,8 @@ int UBRing::UbrTrxSend(const void *buf, uint32_t buf_len) uint32_t total_send_len = 0; uint32_t remain_buf_len = buf_len; uint8_t is_last_pkt = 0; - _trx->ubr_tx.out_io_id++; - ((UbrEventQMsg *)_trx->ubr_tx.remote_rx_event_q.addr)->io_id = _trx->ubr_tx.out_io_id; + const uint64_t io_seq = ATOMIC_ADD(_trx->ubr_tx.out_io_id, 1) + 1; + ((UbrEventQMsg *)_trx->ubr_tx.remote_rx_event_q.addr)->io_id = io_seq; while (remain_buf_len > 0) { is_last_pkt = (uint8_t)(remain_buf_len <= UBR_MSG_PAYLOAD_LEN); msg->header[UBR_MSG_FLAG_INDEX] = is_last_pkt ? UBR_MSG_CHUNK_EOF : UBR_MSG_CHUNK_EXIST; @@ -389,7 +707,7 @@ int UBRing::UbrTrxRecv(void *buf, uint32_t buf_len) return UBRING_ERR; } RETURN_CODE rc = UBRING_OK; - if (UNLIKELY((rc = CheckTrxRecvParam(_trx, buf, buf_len)) != UBRING_OK)) { + if (BAIDU_UNLIKELY((rc = CheckTrxRecvParam(_trx, buf, buf_len)) != UBRING_OK)) { return (rc == UBR_NOT_CONNECTED) ? 0 : rc; } UbrMsgFormat *data_msg = (UbrMsgFormat *)_trx->ubr_rx.local_data_q.addr; @@ -408,7 +726,7 @@ int UBRing::UbrTrxRecvBlockMode(uint8_t *dest, uint32_t buf_len) return UBRING_ERR; } RETURN_CODE rc = UBRING_OK; - if (UNLIKELY((rc = CheckTrxRecvParam(_trx, dest, buf_len)) != UBRING_OK)) { + if (BAIDU_UNLIKELY((rc = CheckTrxRecvParam(_trx, dest, buf_len)) != UBRING_OK)) { return (rc == UBR_NOT_CONNECTED) ? 0 : rc; } @@ -421,7 +739,7 @@ int UBRing::UbrTrxRecvBlockMode(uint8_t *dest, uint32_t buf_len) bool need_update_epoll_eof_pos = ubr_rx->read_pos == ubr_rx->ep_eof_pos; while (not_eof_encountered && remaining_len > 0) { - if (UNLIKELY(CheckTrxRecvPreCheck(_trx) != UBRING_OK)) { + if (BAIDU_UNLIKELY(CheckTrxRecvPreCheck(_trx) != UBRING_OK)) { return UBRING_ERR; } UbrMsgFormat *current_chunk = &data_msg[ubr_rx->read_pos]; @@ -438,7 +756,7 @@ int UBRing::UbrTrxRecvBlockMode(uint8_t *dest, uint32_t buf_len) } uint8_t chunk_msg_len = current_chunk->header[UBR_MSG_LEN_INDEX]; uint8_t cur_index = current_chunk->header[UBR_MSG_CUR_INDEX]; - if (UNLIKELY(!IsRecvChunkHeaderValid(chunk_msg_len, cur_index))) { + if (BAIDU_UNLIKELY(!IsRecvChunkHeaderValid(chunk_msg_len, cur_index))) { LOG(ERROR) << "Trx recv failed, invalid chunk header msg_len=" << (uint32_t)chunk_msg_len << " cur_index=" << (uint32_t)cur_index; errno = EBADMSG; @@ -451,7 +769,7 @@ int UBRing::UbrTrxRecvBlockMode(uint8_t *dest, uint32_t buf_len) total_copied += copy_len; remaining_len -= copy_len; current_chunk->header[UBR_MSG_CUR_INDEX] += (uint8_t)copy_len; - if (LIKELY(current_chunk->header[UBR_MSG_CUR_INDEX] == chunk_msg_len)) { + if (BAIDU_LIKELY(current_chunk->header[UBR_MSG_CUR_INDEX] == chunk_msg_len)) { current_chunk->header[UBR_MSG_FLAG_INDEX] = UBR_MSG_CHUNK_NONE; UpdateDataQTail(_trx); ubr_rx->read_pos = (ubr_rx->read_pos + 1) % ubr_rx->capacity; @@ -549,7 +867,7 @@ ssize_t UBRing::UbrIpcV2Readv(const struct iovec* iov, int iovcnt) ssize_t UBRing::UbrTrxWritev(const struct iovec *iov, int iovcnt) { - if (UNLIKELY(CheckTrxSendPreCheck(_trx) != UBRING_OK)) { + if (BAIDU_UNLIKELY(CheckTrxSendPreCheck(_trx) != UBRING_OK)) { return UBRING_ERR; } if (_trx->data_format == UBR_DATA_FORMAT_IPC_V2) { @@ -579,8 +897,8 @@ ssize_t UBRing::UbrTrxWritev(const struct iovec *iov, int iovcnt) size_t fulled = 0; uint8_t is_last_pkt = 0; uint8_t cur_pkt_len = 0; - _trx->ubr_tx.out_io_id++; - ((UbrEventQMsg *)_trx->ubr_tx.remote_rx_event_q.addr)->io_id = _trx->ubr_tx.out_io_id; + const uint64_t io_seq = ATOMIC_ADD(_trx->ubr_tx.out_io_id, 1) + 1; + ((UbrEventQMsg *)_trx->ubr_tx.remote_rx_event_q.addr)->io_id = io_seq; while (buf_len > 0) { is_last_pkt = (uint8_t)(buf_len <= UBR_MSG_PAYLOAD_LEN); cur_pkt_len = is_last_pkt ? (uint8_t)buf_len : UBR_MSG_PAYLOAD_LEN; @@ -613,7 +931,7 @@ ssize_t UBRing::UbrTrxWritev(const struct iovec *iov, int iovcnt) ssize_t UBRing::UbrTrxReadv(const struct iovec *iov, int iovcnt) { RETURN_CODE rc = UBRING_OK; - if (UNLIKELY((rc = CheckTrxRecvParam(_trx, iov, (uint32_t)iovcnt)) != UBRING_OK)) { + if (BAIDU_UNLIKELY((rc = CheckTrxRecvParam(_trx, iov, (uint32_t)iovcnt)) != UBRING_OK)) { return (rc == UBR_NOT_CONNECTED) ? 0 : rc; } if (_trx->data_format == UBR_DATA_FORMAT_IPC_V2) { @@ -631,7 +949,7 @@ ssize_t UBRing::UbrTrxReadv(const struct iovec *iov, int iovcnt) return -1; } ssize_t nr = UbrTrxReadvBlockMode(iov, iovcnt); - if (UNLIKELY(nr == -1)) { + if (BAIDU_UNLIKELY(nr == -1)) { LOG(ERROR) << "Non-blocking readv msg in failed, connection has been closed."; errno = EPIPE; return -1; @@ -646,7 +964,7 @@ ssize_t UBRing::UbrTrxReadvBlockMode(const struct iovec *iov, int iovcnt) return UBRING_ERR; } RETURN_CODE rc = UBRING_OK; - if (UNLIKELY((rc = CheckTrxRecvParam(_trx, iov, (uint32_t)iovcnt)) != UBRING_OK)) { + if (BAIDU_UNLIKELY((rc = CheckTrxRecvParam(_trx, iov, (uint32_t)iovcnt)) != UBRING_OK)) { return (rc == UBR_NOT_CONNECTED) ? 0 : rc; } @@ -666,15 +984,15 @@ ssize_t UBRing::UbrTrxReadvBlockMode(const struct iovec *iov, int iovcnt) RETURN_CODE UBRing::IsUbrTrxReadable(uint32_t ep_event) { - if (UNLIKELY(_trx == nullptr)) { + if (BAIDU_UNLIKELY(_trx == nullptr)) { LOG(ERROR) << "The trx to be checked is NULL."; return UBRING_ERR; } - if (UNLIKELY(_trx->local_shm.addr == nullptr)) { + if (BAIDU_UNLIKELY(_trx->local_shm.addr == nullptr)) { LOG(ERROR) << "The trx local_shm to be checked is NULL."; return UBRING_ERR; } - if (UNLIKELY(_trx->ubr_tx.trx_state != UBR_STATE_CONNECTED)) { + if (BAIDU_UNLIKELY(_trx->ubr_tx.trx_state != UBR_STATE_CONNECTED)) { return UBRING_ERR; } if (_trx->data_format == UBR_DATA_FORMAT_IPC_V2) { @@ -685,7 +1003,7 @@ RETURN_CODE UBRing::IsUbrTrxReadable(uint32_t ep_event) } uint64_t io_id = ((UbrEventQMsg *)_trx->ubr_rx.local_rx_event_q.addr)->io_id; - if ((ep_event & EPOLLET) && io_id == _trx->ubr_rx.in_io_id) { + if ((ep_event & EPOLLET) && io_id == ATOMIC_LOAD(_trx->ubr_rx.in_io_id)) { return MPA_MUXER_NOT_READY; } @@ -700,31 +1018,31 @@ RETURN_CODE UBRing::IsUbrTrxReadable(uint32_t ep_event) return MPA_MUXER_NOT_READY; } if (ep_event & EPOLLET) { - _trx->ubr_rx.in_io_id = io_id; + ATOMIC_STORE(_trx->ubr_rx.in_io_id, io_id); } return UBRING_OK; } RETURN_CODE UBRing::IsUbrTrxWriteable(uint32_t ep_event) { - if (UNLIKELY(_trx == nullptr)) { + if (BAIDU_UNLIKELY(_trx == nullptr)) { LOG(ERROR) << "The trx to be checked is NULL."; return UBRING_ERR; } - if (UNLIKELY(_trx->local_shm.addr == nullptr)) { + if (BAIDU_UNLIKELY(_trx->local_shm.addr == nullptr)) { LOG(ERROR) << "The trx local_shm to be checked is NULL."; return UBRING_ERR; } - if (UNLIKELY((UbrEventQMsg *)_trx->ubr_tx.local_tx_event_q.addr == nullptr)) { + if (BAIDU_UNLIKELY((UbrEventQMsg *)_trx->ubr_tx.local_tx_event_q.addr == nullptr)) { LOG(ERROR) << "The trx local_tx_event_q addr is NULL."; return UBRING_ERR; } - if (UNLIKELY((UbrEventQMsg *)_trx->ubr_tx.local_data_status_q.addr == nullptr)) { + if (BAIDU_UNLIKELY((UbrEventQMsg *)_trx->ubr_tx.local_data_status_q.addr == nullptr)) { LOG(ERROR) << "The trx local_data_status_q addr is NULL."; return UBRING_ERR; } - if (UNLIKELY(_trx->ubr_tx.trx_state != UBR_STATE_CONNECTED)) { + if (BAIDU_UNLIKELY(_trx->ubr_tx.trx_state != UBR_STATE_CONNECTED)) { LOG(ERROR) << "The trx is not connected state."; return UBRING_ERR; } @@ -850,13 +1168,13 @@ RETURN_CODE UBRing::UbrTrxFreeShm(UbrTrx *trx) RETURN_CODE rc = UBRING_OK; rc = ShmMunmap(&trx->local_shm); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Trx close, local unmap " << trx->local_shm.name << " shm fail."; return UBRING_ERR; } rc = ShmFree(&trx->local_shm); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { if (rc != SHM_ERR_RESOURCE_ATTACHED && rc != SHM_ERR_NOT_FOUND) { LOG(ERROR) << "Wait for " << trx->local_shm.name << " local shm free fail."; return UBRING_ERR; @@ -877,7 +1195,7 @@ RETURN_CODE UBRing::UbrTrxFreeShm(UbrTrx *trx) RETURN_CODE UBRing::UbrUnlinkLocalShm() { - if (UNLIKELY(_trx == nullptr)) { + if (BAIDU_UNLIKELY(_trx == nullptr)) { return UBRING_ERR; } RETURN_CODE rc = ShmFree(&_trx->local_shm); @@ -930,11 +1248,11 @@ void UBRing::PrewriteUbrRx(UbrRx *rx) RETURN_CODE UBRing::UbrTrxMapLocalShm(SHM *local_shm) { - if (UNLIKELY(_trx == nullptr)) { + if (BAIDU_UNLIKELY(_trx == nullptr)) { LOG(ERROR) << "Trx map Shared memory failed, trx is null."; return UBRING_ERR; } - if (UNLIKELY(local_shm == nullptr || local_shm->addr == nullptr)) { + if (BAIDU_UNLIKELY(local_shm == nullptr || local_shm->addr == nullptr)) { LOG(ERROR) << "Trx map Shared memory failed, local_shm is null or addr is NULL."; return UBRING_ERR; } @@ -953,11 +1271,11 @@ RETURN_CODE UBRing::UbrTrxMapLocalShm(SHM *local_shm) RETURN_CODE UBRing::UbrTrxMapRemoteShm(SHM *remote_shm) { - if (UNLIKELY(_trx == nullptr)) { + if (BAIDU_UNLIKELY(_trx == nullptr)) { LOG(ERROR) << "Trx map Shared memory failed, trx is null."; return UBRING_ERR; } - if (UNLIKELY(remote_shm == nullptr || remote_shm->addr == nullptr)) { + if (BAIDU_UNLIKELY(remote_shm == nullptr || remote_shm->addr == nullptr)) { LOG(ERROR) << "Trx map Shared memory failed, remote_shm is null or addr is NULL."; return UBRING_ERR; } @@ -982,7 +1300,7 @@ RETURN_CODE UBRing::InitializeServerLegacyFormat() _trx->ubr_tx.capacity = remote_data_msg_cap; RETURN_CODE rc = UBRingManager::GetUbrDealMsgMaxCnt( _trx->ubr_rx.capacity, &_trx->ubr_rx.deal_msg_max_cnt); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Get ubring deal msg max cnt."; return rc; } @@ -998,7 +1316,7 @@ RETURN_CODE UBRing::UbrServerTrxInit(SHM* local_shm, SHM* remote_shm, UbrDataFormat format) { RETURN_CODE rc = UbrTrxMapShm(local_shm, remote_shm); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Trx map shared memory failed."; return rc; } @@ -1010,11 +1328,11 @@ RETURN_CODE UBRing::UbrServerTrxInit(SHM* local_shm, SHM* remote_shm, } else { rc = UBRING_ERR; } - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { return rc; } - if (UNLIKELY(UbrAddTimer() != UBRING_OK)) { + if (BAIDU_UNLIKELY(UbrAddTimer() != UBRING_OK)) { LOG(ERROR) << "Ubr add timer failed, local_name=" << local_shm->name; return UBRING_ERR; } @@ -1081,30 +1399,30 @@ int UBRing::UbrPrepareIpcV2Format() int UBRing::UbrAllocateServerShm(SHM* remote_trx_shm, SHM* local_trx_shm, UbrDataFormat format) { UbrSetSleepTask(UBR_TASK_ACCEPT_MAP_FRONT); - if (UNLIKELY((ShmRemoteMalloc(remote_trx_shm)) != UBRING_OK)) { + if (BAIDU_UNLIKELY((ShmRemoteMalloc(remote_trx_shm)) != UBRING_OK)) { LOG(ERROR) << "Trx apply remote shared memory failed."; return -1; } - if (UNLIKELY((ShmLocalCalloc(local_trx_shm)) != UBRING_OK)) { + if (BAIDU_UNLIKELY((ShmLocalCalloc(local_trx_shm)) != UBRING_OK)) { LOG(ERROR) << "Trx apply local shared memory failed."; ShmRemoteFree(remote_trx_shm); return -1; } UbrTrx **ubr_trx_ptr = &_trx; - if (UNLIKELY((UBRingManager::AcquireUbrTrxFromMgr(ubr_trx_ptr)) != UBRING_OK)) { + if (BAIDU_UNLIKELY((UBRingManager::AcquireUbrTrxFromMgr(ubr_trx_ptr)) != UBRING_OK)) { LOG(ERROR) << "Acquire ubrtrx failed."; ShmRemoteFree(remote_trx_shm); ShmLocalFree(local_trx_shm); return -1; } _trx->type = TCP_TRX; - if (UNLIKELY((UbrServerTrxInit( + if (BAIDU_UNLIKELY((UbrServerTrxInit( local_trx_shm, remote_trx_shm, format)) != UBRING_OK)) { LOG(ERROR) << "Server trx init failed."; UbrTrxFreeShm(_trx); - UBRingManager::ReleaseUbrTrxFromMgr(_trx); + UBRingManager::ReleaseUbrTrxFromMgr(_trx, ATOMIC_LOAD(_trx->ubr_id)); _trx = nullptr; return -1; } @@ -1113,14 +1431,14 @@ int UBRing::UbrAllocateServerShm(SHM* remote_trx_shm, SHM* local_trx_shm, int UBRing::UbrAllocateLocalShm(SHM *local_trx_shm, const char *shm_name) { - if (UNLIKELY((UBRingManager::AcquireUbrTrxFromMgr(&(_trx))) != UBRING_OK)) { + if (BAIDU_UNLIKELY((UBRingManager::AcquireUbrTrxFromMgr(&(_trx))) != UBRING_OK)) { LOG(ERROR) << "Acquire ubrtrx failed, local_name=" << shm_name; return -1; } _trx->type = TCP_TRX; RETURN_CODE rc = ApplyAndMapLocalShm(local_trx_shm, shm_name); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Trx apply or map local shared memory failed, local_name=" << shm_name; _trx = nullptr; return -1; @@ -1133,7 +1451,7 @@ int UBRing::UbrMapRemoteShm(SHM *local_trx_shm, const char *local_name, { RETURN_CODE rc = UbrMapRemoteShmAddTimer( local_trx_shm, local_name, remote_shm_len, format); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Connect Trx failed, local shm name=" << local_trx_shm->name; return -1; } @@ -1154,7 +1472,7 @@ RETURN_CODE UBRing::InitializeClientLocalLegacyFormat() (uint32_t)(_trx->ubr_rx.local_data_q.len / UBR_MSG_LEN); RETURN_CODE rc = UBRingManager::GetUbrDealMsgMaxCnt( _trx->ubr_rx.capacity, &_trx->ubr_rx.deal_msg_max_cnt); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Get ubring deal msg max cnt."; return rc; } @@ -1199,20 +1517,21 @@ RETURN_CODE UBRing::UbrMapRemoteShmAddTimer(SHM *local_trx_shm, const char *loca SHM_NAME_PREFIX, local_name, SERVER_SHM_NAME_SUFFIX); - if (UNLIKELY(result < 0)) { - LOG(ERROR) << "Copy server shared memory name failed, local_name=%s, ret=%d.", local_name, result; + if (BAIDU_UNLIKELY(result < 0)) { + LOG(ERROR) << "Copy server shared memory name failed, local_name=" << local_name + << ", ret=" << result; return UBRING_ERR; } UbrSetSleepTask(UBR_TASK_CONNECT_MAP_FRONT); RETURN_CODE rc = ApplyAndMapRemoteShm(&remote_trx_shm); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Connect Trx map shared memory failed, remote shm=" << remote_trx_shm.name; return rc; } if (format == UBR_DATA_FORMAT_LEGACY_64) { InitializeClientRemoteLegacyFormat(); rc = InitializeClientLocalLegacyFormat(); - if (LIKELY(rc == UBRING_OK)) { + if (BAIDU_LIKELY(rc == UBRING_OK)) { InitializeClientLegacyFormat(); } } else if (format == UBR_DATA_FORMAT_IPC_V2) { @@ -1220,14 +1539,17 @@ RETURN_CODE UBRing::UbrMapRemoteShmAddTimer(SHM *local_trx_shm, const char *loca } else { rc = UBRING_ERR; } - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Initialize client data format failed, local_name=" << local_name << ", format=" << format; return rc; } - if (UNLIKELY(UbrAddTimer() != UBRING_OK)) { + if (BAIDU_UNLIKELY(UbrAddTimer() != UBRING_OK)) { LOG(ERROR) << "Ubr add timer failed, local_name=" << local_name; + // The trx slot stays acquired on purpose: the endpoint keeps this + // UBRing (it falls back to TCP) and returns the slot through its own + // Reset()/UbrTrxClose path. Releasing it here would race that owner. ShmRemoteFree(&_trx->remote_shm); return UBRING_ERR; } @@ -1237,8 +1559,10 @@ RETURN_CODE UBRing::UbrMapRemoteShmAddTimer(SHM *local_trx_shm, const char *loca uint32_t timeout = ((UbrDataStatusQMsg *)(_trx->ubr_tx.local_data_status_q.addr))->timeout; if (HasTimedOut(start_time, timeout) != UBRING_OK) { LOG(ERROR) << "Local shm " << local_trx_shm->name << " wait for connect remote map timeout."; - DeleteTimerSafe((uint32_t)_trx->hb_timer_fd); - DeleteTimerSafe((uint32_t)_trx->timer_fd); + UbrTimerDelAndWait(&_trx->hb_timer); + UbrTimerDelAndWait(&_trx->close_timer); + // Same ownership as above: the slot is returned by the endpoint's + // close path, not here. ShmRemoteFree(&_trx->remote_shm); return UBRING_ERR_TIMEOUT; } @@ -1250,7 +1574,7 @@ RETURN_CODE UBRing::UbrMapRemoteShmAddTimer(SHM *local_trx_shm, const char *loca RETURN_CODE UBRing::ApplyAndMapLocalShm(SHM *local_trx_shm, const char *local_name) { - if (UNLIKELY(_trx == nullptr || local_trx_shm == nullptr)) { + if (BAIDU_UNLIKELY(_trx == nullptr || local_trx_shm == nullptr)) { LOG(ERROR) << "Trx map Shared memory failed, trx is null, local_name=" << local_name; return UBRING_ERR; } @@ -1260,25 +1584,25 @@ RETURN_CODE UBRing::ApplyAndMapLocalShm(SHM *local_trx_shm, const char *local_na SHM_NAME_PREFIX, local_name, CLIENT_SHM_NAME_SUFFIX); - if (UNLIKELY(result < 0)) { + if (BAIDU_UNLIKELY(result < 0)) { LOG(ERROR) << "Copy client localTrx shared memory name failed, local_name=" << local_name << ", ret=" << result; return UBRING_ERR; } RETURN_CODE rc = ShmLocalCalloc(local_trx_shm); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Trx apply local shared memory failed, local shm name=" << local_trx_shm->name << ", rc=" << rc; if (rc == SHM_ERR_EXIST || rc == SHM_ERR_NOT_FOUND) { rc = UBR_ERR_ADDR_IN_USE; } - UBRingManager::ReleaseUbrTrxFromMgr(_trx); + UBRingManager::ReleaseUbrTrxFromMgr(_trx, ATOMIC_LOAD(_trx->ubr_id)); return rc; } rc = UbrTrxMapLocalShm(local_trx_shm); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Trx map local shared memory failed, local shm name=" << local_trx_shm->name; ShmLocalFree(local_trx_shm); - UBRingManager::ReleaseUbrTrxFromMgr(_trx); + UBRingManager::ReleaseUbrTrxFromMgr(_trx, ATOMIC_LOAD(_trx->ubr_id)); return rc; } ((UbrDataStatusQMsg *)_trx->ubr_tx.local_data_status_q.addr)->timeout = @@ -1289,12 +1613,12 @@ RETURN_CODE UBRing::ApplyAndMapLocalShm(SHM *local_trx_shm, const char *local_na RETURN_CODE UBRing::ApplyAndMapRemoteShm(SHM *remote_trx_shm) { RETURN_CODE rc = ShmRemoteMalloc(remote_trx_shm); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Trx apply remote shared memory failed."; return rc; } rc = UbrTrxMapRemoteShm(remote_trx_shm); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Trx map shared memory failed."; ShmRemoteFree(remote_trx_shm); return rc; @@ -1322,24 +1646,27 @@ RETURN_CODE UBRing::WritevHasEnoughSpace(size_t buf_len) return UBRING_OK; } -RETURN_CODE UBRing::UbrClearResourceCheck(UbrTrx *trx, uint64_t start_time, UbrCloseType close_type) +RETURN_CODE UBRing::UbrClearResourceCheck(UbrTrx *trx) { - if (UNLIKELY(trx == nullptr)) { + if (BAIDU_UNLIKELY(trx == nullptr)) { LOG(ERROR) << "Trx close failed, trx is null."; return UBRING_ERR; } UbrEventQMsg* local_tx_event_q = (UbrEventQMsg *)trx->ubr_tx.local_tx_event_q.addr; + if (BAIDU_UNLIKELY(local_tx_event_q == nullptr)) { + LOG(ERROR) << "Trx close failed, local_tx_event_q addr is NULL, trx local name=" << trx->local_shm.name; + return UBRING_ERR; + } if (local_tx_event_q->flag == UBR_STATE_CONNECTED) { local_tx_event_q->flag = UBR_STATE_CLOSING; } - if (close_type == UBR_SEND_CLOSE) { - DeleteTimerSafe((uint32_t)trx->timer_fd); - } else { - DeleteTimer((uint32_t)trx->timer_fd); - } - DeleteTimerSafe((uint32_t)trx->hb_timer_fd); + // Wait out a dispatched callback before the slot's trx is cleared. When + // this runs inside a per-trx callback, UbrTimerDelAndWait recognizes its own + // timer and degrades to the non-blocking delete. + UbrStopTrxTimer(&trx->close_timer); + UbrStopTrxTimer(&trx->hb_timer); if (local_tx_event_q->flag == UBR_STATE_CLOSING) { local_tx_event_q->flag = UBR_STATE_CLOSED; @@ -1349,14 +1676,14 @@ RETURN_CODE UBRing::UbrClearResourceCheck(UbrTrx *trx, uint64_t start_time, UbrC return UBRING_OK; } -RETURN_CODE UBRing::ClearTrxResource(UbrTrx *trx, uint64_t start_time, UbrCloseType close_type, int op) +RETURN_CODE UBRing::ClearTrxResource(UbrTrx *trx, uint64_t expect_ubr_id) { - RETURN_CODE rc = UbrClearResourceCheck(trx, start_time, close_type); + RETURN_CODE rc = UbrClearResourceCheck(trx); if (rc != UBRING_OK) { return rc; } - rc = UbrAddAsynClearTimer(trx); + rc = UbrAddAsynClearTimer(trx, expect_ubr_id); if (rc != UBRING_OK) { LOG(ERROR) << "Trx close, add " << trx->local_shm.name << " close clear timer failed."; return UBRING_ERR; @@ -1365,23 +1692,21 @@ RETURN_CODE UBRing::ClearTrxResource(UbrTrx *trx, uint64_t start_time, UbrCloseT return UBRING_OK; } -RETURN_CODE UBRing::UbrTrxCloseCheck(UbrTrx *trx) +RETURN_CODE UBRing::UbrTrxCloseCheck(UbrTrx *trx, uint64_t expect_ubr_id) { - if (UNLIKELY(trx == nullptr)) { + if (BAIDU_UNLIKELY(trx == nullptr)) { LOG(ERROR) << "Trx close failed, client trx is null."; return UBRING_ERR; } - int expected = MAX_CLOSE_COUNT; - if (!ATOMIC_COMPARE_EXCHANGE_STRONG(trx->close_cnt, expected, MAX_CLOSE_COUNT - 1)) { + // Validate the generation and claim the close in one critical section: + // checking here and claiming afterwards would let a slot released and + // reused in between be closed as if it were still this generation's. + RETURN_CODE rc = UBRingManager::TryClaimTrxClose(trx->trx_mgr_index, + expect_ubr_id); + if (rc == UBRING_REENTRY) { LOG(INFO) << "Trx close skipped, already closing, trx local name=" << trx->local_shm.name; - return UBRING_REENTRY; - } - - if (UNLIKELY(trx->ubr_tx.local_tx_event_q.addr == nullptr)) { - LOG(ERROR) << "Trx close failed, local_tx_event_q addr is NULL, trx local name=" << trx->local_shm.name; - return UBRING_ERR; } - return UBRING_OK; + return rc; } ssize_t UBRing::StartReadv(UbrTrx *trx, const struct iovec *iov, int iovcnt, size_t remain_buf_len) @@ -1392,7 +1717,7 @@ ssize_t UBRing::StartReadv(UbrTrx *trx, const struct iovec *iov, int iovcnt, siz UbrMsgFormat *data_msg = (UbrMsgFormat *)trx->ubr_rx.local_data_q.addr; bool not_eof_encountered = true; while (not_eof_encountered && remain_buf_len > 0) { - if (UNLIKELY(CheckTrxRecvPreCheck(trx) != UBRING_OK)) { + if (BAIDU_UNLIKELY(CheckTrxRecvPreCheck(trx) != UBRING_OK)) { return UBRING_ERR; } UbrMsgFormat *current_chunk = &data_msg[trx->ubr_rx.read_pos]; @@ -1409,7 +1734,7 @@ ssize_t UBRing::StartReadv(UbrTrx *trx, const struct iovec *iov, int iovcnt, siz } uint8_t chunk_msg_len = current_chunk->header[UBR_MSG_LEN_INDEX]; uint8_t cur_index = current_chunk->header[UBR_MSG_CUR_INDEX]; - if (UNLIKELY(!IsRecvChunkHeaderValid(chunk_msg_len, cur_index))) { + if (BAIDU_UNLIKELY(!IsRecvChunkHeaderValid(chunk_msg_len, cur_index))) { LOG(ERROR) << "Trx readv failed, invalid chunk header msg_len=" << (uint32_t)chunk_msg_len << " cur_index=" << (uint32_t)cur_index; errno = EBADMSG; diff --git a/src/brpc/ubshm/ub_ring.h b/src/brpc/ubshm/ub_ring.h index bd667d1f3f..52d773f77a 100644 --- a/src/brpc/ubshm/ub_ring.h +++ b/src/brpc/ubshm/ub_ring.h @@ -45,21 +45,26 @@ class UBRing : public butil::IReader { RETURN_CODE UbrTrxClose(); - RETURN_CODE UbrAddCloseTimer(); - + // Arms both per-trx timers. The only entry point for them: it holds the + // manager's arming exclusion, which UbrMgrFini relies on to know that no + // new timer can appear after it took the shutdown flag. RETURN_CODE UbrAddTimer(); - static void *UbrTrxCloseCallback(void *args); + static void *UbrTrxCloseCallback(void *args, uint64_t gen); - RETURN_CODE UbrAddHBTimer(); + static void *UbrTrxHBCallback(void *args, uint64_t gen); - static void *UbrTrxHBCallback(void *args); + // Teardown entry point for a per-trx timer callback and for the faulty-shm + // event. `expect_ubr_id' is the generation the caller saw; the close is + // claimed atomically for that generation, so a slot released and reused in + // between is left alone. + static RETURN_CODE UbrPassiveClearTrx(UbrTrx *trx, uint64_t expect_ubr_id); - static RETURN_CODE UbrPassiveClearTrx(UbrTrx *trx, int fd, PASSIVE_DISC_TYPE type); + static RETURN_CODE UbrAddAsynClearTimer(UbrTrx *trx, uint64_t expect_ubr_id); - static RETURN_CODE UbrAddAsynClearTimer(UbrTrx *trx); + static void *UbrAsynClearCallback(void *args, uint64_t gen); - static void *UbrAsynClearCallback(void *args); + static void *UbrPassiveClearCallback(void *args, uint64_t gen); int UbrTrxSend(const void *buf, uint32_t buf_len); @@ -96,11 +101,11 @@ class UBRing : public butil::IReader { static inline RETURN_CODE CheckTrxConnectParam(const char *listener_name, const char *local_name) { - if (UNLIKELY(listener_name == nullptr)) { + if (BAIDU_UNLIKELY(listener_name == nullptr)) { LOG(ERROR) << "The request listener name is null."; return UBRING_ERR; } - if (UNLIKELY(local_name == nullptr)) { + if (BAIDU_UNLIKELY(local_name == nullptr)) { LOG(ERROR) << "The request trx shared memory name is null."; return UBRING_ERR; } @@ -133,11 +138,11 @@ class UBRing : public butil::IReader { static inline RETURN_CODE CheckTrxSendPreCheck(UbrTrx *trx) { - if (UNLIKELY(trx == nullptr)) { + if (BAIDU_UNLIKELY(trx == nullptr)) { LOG(ERROR) << "Trx send failed, trx is null."; return UBRING_ERR; } - if (UNLIKELY(trx->ubr_tx.trx_state != UBR_STATE_CONNECTED)) { + if (BAIDU_UNLIKELY(trx->ubr_tx.trx_state != UBR_STATE_CONNECTED)) { LOG(ERROR) << "Trx send failed, trx is not connected state."; return UBRING_ERR; } @@ -146,25 +151,25 @@ class UBRing : public butil::IReader { } static RETURN_CODE CheckTrxRecvParam(UbrTrx *trx, const void *buf, uint32_t buf_len) { - if (UNLIKELY(trx == nullptr)) { + if (BAIDU_UNLIKELY(trx == nullptr)) { LOG(ERROR) << "Trx recv failed, trx is null."; return UBRING_ERR; } - if (UNLIKELY((UbrEventQMsg *)trx->ubr_rx.local_rx_event_q.addr == nullptr)) { + if (BAIDU_UNLIKELY((UbrEventQMsg *)trx->ubr_rx.local_rx_event_q.addr == nullptr)) { LOG(ERROR) << "Trx send failed, local_tx_event_q addr is NULL."; return UBRING_ERR; } - if (UNLIKELY(trx->ubr_rx.trx_state != UBR_STATE_CONNECTED)) { + if (BAIDU_UNLIKELY(trx->ubr_rx.trx_state != UBR_STATE_CONNECTED)) { LOG(ERROR) << "Trx recv failed, trx is not connected statep=" << trx->ubr_rx.trx_state; return UBR_NOT_CONNECTED; } - if (UNLIKELY(buf == nullptr)) { + if (BAIDU_UNLIKELY(buf == nullptr)) { LOG(ERROR) << "Trx recv failed, buf is null."; return UBRING_ERR; } - if (UNLIKELY(buf_len == 0)) { + if (BAIDU_UNLIKELY(buf_len == 0)) { LOG(ERROR) << "Trx recv failed, buf_len is 0."; return UBRING_ERR; } @@ -173,7 +178,7 @@ class UBRing : public butil::IReader { static inline RETURN_CODE CheckTrxRecvPreCheck(UbrTrx *trx) { - if (UNLIKELY(trx->ubr_rx.trx_state != UBR_STATE_CONNECTED)) { + if (BAIDU_UNLIKELY(trx->ubr_rx.trx_state != UBR_STATE_CONNECTED)) { LOG(ERROR) << "Trx recv failed, trx is not connected state."; return UBRING_ERR; } @@ -206,15 +211,15 @@ class UBRing : public butil::IReader { LOG(ERROR) << "Trx close callback failed, trx is null."; return UBRING_ERR; } - if (UNLIKELY(trx->local_shm.addr == nullptr)) { + if (BAIDU_UNLIKELY(trx->local_shm.addr == nullptr)) { LOG(ERROR) << "Trx close failed, local_shm addr is NULL."; return UBRING_ERR; } - if (UNLIKELY(trx->ubr_rx.local_rx_event_q.addr == nullptr)) { + if (BAIDU_UNLIKELY(trx->ubr_rx.local_rx_event_q.addr == nullptr)) { LOG(ERROR) << "Trx close failed, local_rx_event_q addr is NULL."; return UBRING_ERR; } - if (UNLIKELY(trx->ubr_tx.local_tx_event_q.addr == nullptr)) { + if (BAIDU_UNLIKELY(trx->ubr_tx.local_tx_event_q.addr == nullptr)) { LOG(ERROR) << "Trx close failed, local_tx_event_q addr is NULL."; return UBRING_ERR; } @@ -226,11 +231,16 @@ class UBRing : public butil::IReader { ssize_t UbrIpcV2Readv(const struct iovec* iov, int iovcnt); RETURN_CODE IsIpcV2Readable(uint32_t ep_event); RETURN_CODE IsIpcV2Writeable(uint32_t ep_event); + // Only UbrAddTimer may call these: arming outside its manager-held + // exclusion would break the UbrMgrFini invariant described above. + RETURN_CODE UbrAddCloseTimer(); + RETURN_CODE UbrAddHBTimer(); + RETURN_CODE UbrTrxMapLocalShm(SHM *local_shm); RETURN_CODE UbrTrxMapRemoteShm(SHM *remote_shm); RETURN_CODE ApplyAndMapLocalShm(SHM *local_trx_shm, const char *local_name); RETURN_CODE ApplyAndMapRemoteShm(SHM *remote_trx_shm); - static RETURN_CODE UbrTrxCloseCheck(UbrTrx *trx); + static RETURN_CODE UbrTrxCloseCheck(UbrTrx *trx, uint64_t expect_ubr_id); void ReleaseFileLock(int lock_fd); ssize_t StartReadv(UbrTrx *trx, const struct iovec *iov, int iovcnt, size_t remain_buf_len); void PreWriteAddr(uint8_t *addr, size_t len); @@ -242,8 +252,8 @@ class UBRing : public butil::IReader { void InitializeClientRemoteLegacyFormat(); void InitializeClientLegacyFormat(); void ActivateClientFormat(); - static RETURN_CODE UbrClearResourceCheck(UbrTrx *trx, uint64_t start_time, UbrCloseType close_type); - static RETURN_CODE ClearTrxResource(UbrTrx *trx, uint64_t start_time, UbrCloseType close_type, int op=0); + static RETURN_CODE UbrClearResourceCheck(UbrTrx *trx); + static RETURN_CODE ClearTrxResource(UbrTrx *trx, uint64_t expect_ubr_id); UbrTrx* _trx{nullptr}; }; diff --git a/src/brpc/ubshm/ub_ring_manager.cpp b/src/brpc/ubshm/ub_ring_manager.cpp index 64f2434eba..c5c654f98b 100644 --- a/src/brpc/ubshm/ub_ring_manager.cpp +++ b/src/brpc/ubshm/ub_ring_manager.cpp @@ -15,13 +15,21 @@ // specific language governing permissions and limitations // under the License. +#include +#include #include #include "brpc/ubshm/ub_ring.h" #include "brpc/ubshm/ub_ring_manager.h" #include "butil/logging.h" +#include "butil/scoped_lock.h" namespace brpc { namespace ubring { + +// A UbrCleanupCtl reference count of 1 means only the manager anchor is +// left, i.e. no cleanup callback is in flight for it. +static constexpr int kAnchoredRefOnly = 1; + DEFINE_int32(ubr_max_managed_num, 1024, "maximum number of managed ubring"); DEFINE_int32(tail_update_after_read, 8, "Position of the tail update after the read"); @@ -36,11 +44,11 @@ uint64_t g_ub_event_cnt = 0; uint64_t g_ubr_listener_num = 0; RETURN_CODE UBRingManager::GetUbrDealMsgMaxCnt(const uint32_t capacity, uint32_t *deal_msg_max_cnt) { - if (UNLIKELY(deal_msg_max_cnt == nullptr)) { + if (BAIDU_UNLIKELY(deal_msg_max_cnt == nullptr)) { LOG(ERROR) << "Get update factor failed, deal_msg_max_cnt is null."; return UBRING_ERR; } - if (UNLIKELY(FLAGS_tail_update_after_read == 0)) { + if (BAIDU_UNLIKELY(FLAGS_tail_update_after_read == 0)) { LOG(ERROR) << "Get update factor failed, factor is 0."; return UBRING_ERR; } @@ -52,14 +60,18 @@ RETURN_CODE UBRingManager::UbrMgrDefault() { g_ubr_mgr.trx_num = 0; g_ubr_mgr.trx_cap = FLAGS_ubr_max_managed_num; + g_ubr_mgr.shutting_down = false; + g_ubr_mgr.active_pool_ops = 0; g_ubr_mgr.trx_mgr_unit_status = nullptr; g_ubr_mgr.trx_mgr = nullptr; + g_ubr_mgr.trx_mgr_unit_id = nullptr; + g_ubr_mgr.trx_mgr_unit_ctl = nullptr; return UBRING_OK; } RETURN_CODE UBRingManager::UbrMgrInit() { RETURN_CODE rc = UbrMgrDefault(); - if (UNLIKELY(rc != UBRING_OK)) { + if (BAIDU_UNLIKELY(rc != UBRING_OK)) { LOG(ERROR) << "Ubr manager set default values failed."; return rc; } @@ -68,27 +80,139 @@ RETURN_CODE UBRingManager::UbrMgrInit() { g_ubr_mgr.trx_mgr = (UbrTrx *)malloc(trx_mgr_size); size_t trx_mgr_status_size = g_ubr_mgr.trx_cap * sizeof(UbrMgrUnitStatus); g_ubr_mgr.trx_mgr_unit_status = (UbrMgrUnitStatus *)malloc(trx_mgr_status_size); - if (UNLIKELY(g_ubr_mgr.trx_mgr == nullptr || - g_ubr_mgr.trx_mgr_unit_status == nullptr)) { + size_t trx_mgr_id_size = g_ubr_mgr.trx_cap * sizeof(uint64_t); + g_ubr_mgr.trx_mgr_unit_id = (uint64_t *)malloc(trx_mgr_id_size); + size_t trx_mgr_ctl_size = g_ubr_mgr.trx_cap * sizeof(UbrCleanupCtl *); + g_ubr_mgr.trx_mgr_unit_ctl = (UbrCleanupCtl **)malloc(trx_mgr_ctl_size); + if (BAIDU_UNLIKELY(g_ubr_mgr.trx_mgr == nullptr || + g_ubr_mgr.trx_mgr_unit_status == nullptr || + g_ubr_mgr.trx_mgr_unit_id == nullptr || + g_ubr_mgr.trx_mgr_unit_ctl == nullptr)) { LOG(ERROR) << "Ubr manager memory allocation failed."; UbrMgrFini(); return UBRING_ERR; } - memset(g_ubr_mgr.trx_mgr, 0, trx_mgr_size); + // UbrTrx holds butil::atomic members, so it is not trivially copyable and + // must not be memset: value-initialize every slot instead. A slot is + // re-initialized by AcquireUbrTrxFromMgr before it is handed out, and no + // code reads a slot that was never acquired. + for (uint32_t i = 0; i < g_ubr_mgr.trx_cap; ++i) { + new (&g_ubr_mgr.trx_mgr[i]) UbrTrx(); + } memset(g_ubr_mgr.trx_mgr_unit_status, UBR_MGR_UNIT_FREE, trx_mgr_status_size); + memset(g_ubr_mgr.trx_mgr_unit_id, 0, trx_mgr_id_size); + memset(g_ubr_mgr.trx_mgr_unit_ctl, 0, trx_mgr_ctl_size); LinkInfoInit(); return UBRING_OK; } void UBRingManager::UbrMgrFini() { + // Refuse new pool users and new acquisitions first, unconditionally: the + // partial-init path below (a failed allocation) must not let a callback + // walk arrays that the failure left null either. { - LOCK_GUARD(g_ubr_trx_mgr_mtx); + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + g_ubr_mgr.shutting_down = true; + } + + // The pool arrays are only safe to walk once UbrMgrInit has allocated and + // zeroed all four of them; its allocation-failure path calls this function + // with some of them still null and the others uninitialized. + const bool pool_ready = g_ubr_mgr.trx_mgr != nullptr && + g_ubr_mgr.trx_mgr_unit_status != nullptr && + g_ubr_mgr.trx_mgr_unit_id != nullptr && + g_ubr_mgr.trx_mgr_unit_ctl != nullptr; + + // Wait out close/heartbeat callbacks that are already dispatched, then + // stop the timers, before the pool is freed: UbrTimerDel alone is + // non-blocking, so a callback that already passed its generation check + // could still be reading UbrTrx when FREE_PTR(trx_mgr) runs. + // UbrTimerDelAndWait blocks until a dispatched callback has returned, but + // it must run without g_ubr_trx_mgr_mtx because the callbacks take that + // lock themselves; snapshot the slots under the lock first. + if (pool_ready) { + std::vector used_slots; + { + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + for (uint32_t i = 0; i < g_ubr_mgr.trx_cap; ++i) { + if (g_ubr_mgr.trx_mgr_unit_status[i] == UBR_MGR_UNIT_USED) { + used_slots.push_back(i); + } + } + } + // Wait for the callback-driven pool accesses that are already running + // (they read UbrTrx without owning a timer), then for the timers. Both + // waits must happen without g_ubr_trx_mgr_mtx: the close/heartbeat + // callbacks and the accesses themselves take that lock. + for (;;) { + { + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + if (g_ubr_mgr.active_pool_ops == 0) { + break; + } + } + usleep(1000); + } + // No timer can be armed after the flag: arming and taking the flag are + // serialized by the manager lock (ArmTimersExclusive), and a cleanup + // control object can no longer be published either + // (TryPublishUnitCleanupCtl refuses once shutting down). So the timers + // armed before the flag are the complete set, and stopping them all + // leaves the pool quiescent. + for (uint32_t i : used_slots) { + UbrTimerDelAndWait(&g_ubr_mgr.trx_mgr[i].close_timer); + UbrTimerDelAndWait(&g_ubr_mgr.trx_mgr[i].hb_timer); + } + } + + // Cancel the pending delayed cleanups and wait for the in-flight ones + // (each holds one extra reference) to finish, before the pool memory + // they touch is freed. A ctl whose timer is still starting can only be + // cancelled in a later round, hence the retry-to-stability loop. + bool busy = true; + while (busy) { + busy = false; + { + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + if (pool_ready) { + for (uint32_t i = 0; i < g_ubr_mgr.trx_cap; ++i) { + UbrCleanupCtl* ctl = g_ubr_mgr.trx_mgr_unit_ctl[i]; + if (ctl == nullptr) { + continue; + } + if (UbrTimerDel(&ctl->timer) == 0) { + ctl->ReleaseRef(); // timer/callback reference + } + if (ctl->ref.load() > kAnchoredRefOnly) { + busy = true; + } + } + } + } + if (busy) { + LOG_EVERY_SECOND(INFO) << "UbrMgrFini waits for in-flight cleanups."; + usleep(1000); + } + } + { + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + if (pool_ready) { + for (uint32_t i = 0; i < g_ubr_mgr.trx_cap; ++i) { + UbrCleanupCtl* ctl = g_ubr_mgr.trx_mgr_unit_ctl[i]; + if (ctl != nullptr) { + g_ubr_mgr.trx_mgr_unit_ctl[i] = nullptr; + ctl->ReleaseRef(); // manager reference + } + } + } FREE_PTR(g_ubr_mgr.trx_mgr); FREE_PTR(g_ubr_mgr.trx_mgr_unit_status); + FREE_PTR(g_ubr_mgr.trx_mgr_unit_id); + FREE_PTR(g_ubr_mgr.trx_mgr_unit_ctl); } { - LOCK_GUARD(g_ubr_listener_mgr_mtx); + BAIDU_SCOPED_LOCK(g_ubr_listener_mgr_mtx); } g_ubr_mgr.trx_num = 0; g_ubr_mgr.trx_cap = 0; @@ -96,17 +220,21 @@ void UBRingManager::UbrMgrFini() { } RETURN_CODE UBRingManager::AcquireUbrTrxFromMgr(UbrTrx **trx) { - if (UNLIKELY(trx == nullptr)) { + if (BAIDU_UNLIKELY(trx == nullptr)) { LOG(ERROR) << "Acquire trx failed, trx is null."; return UBRING_ERR; } - if (UNLIKELY(g_ubr_mgr.trx_mgr == nullptr)) { + if (BAIDU_UNLIKELY(g_ubr_mgr.trx_mgr == nullptr)) { LOG(ERROR) << "Acquire trx failed, trx_mgr is null."; return UBRING_ERR; } - LOCK_GUARD(g_ubr_trx_mgr_mtx); + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + if (BAIDU_UNLIKELY(g_ubr_mgr.shutting_down)) { + LOG(ERROR) << "Acquire trx failed, ubr manager is shutting down."; + return UBRING_ERR; + } if (g_ubr_mgr.trx_num >= g_ubr_mgr.trx_cap) { LOG(ERROR) << "Acquire trx failed, trx number is full."; return UBRING_ERR; @@ -114,11 +242,30 @@ RETURN_CODE UBRingManager::AcquireUbrTrxFromMgr(UbrTrx **trx) { for (uint32_t i = 0; i < g_ubr_mgr.trx_cap; ++i) { if (g_ubr_mgr.trx_mgr_unit_status[i] == UBR_MGR_UNIT_FREE) { - memset(&g_ubr_mgr.trx_mgr[i], 0, sizeof(UbrTrx)); + // Value-initialize the slot so its butil::atomic members are + // properly constructed while every other field is zeroed, which + // is what the previous memset did. + new (&g_ubr_mgr.trx_mgr[i]) UbrTrx(); + // The explicit re-initialization is deliberate: it documents the + // per-acquisition invariants of these fields. + g_ubr_mgr.trx_mgr[i].close_timer = nullptr; + g_ubr_mgr.trx_mgr[i].hb_timer = nullptr; + g_ubr_mgr.trx_mgr[i].cleanup_ctl = nullptr; + g_ubr_mgr.trx_mgr[i].cleanup_forced.store(false); + // Retire the previous acquisition's cleanup control object. + UbrCleanupCtl* old_ctl = g_ubr_mgr.trx_mgr_unit_ctl[i]; + g_ubr_mgr.trx_mgr_unit_ctl[i] = nullptr; + if (old_ctl != nullptr) { + if (UbrTimerDel(&old_ctl->timer) == 0) { + old_ctl->ReleaseRef(); // timer/callback reference + } + old_ctl->ReleaseRef(); // manager anchor + } g_ubr_mgr.trx_mgr_unit_status[i] = UBR_MGR_UNIT_USED; *trx = &g_ubr_mgr.trx_mgr[i]; (*trx)->trx_mgr_index = i; - (*trx)->ubr_id = g_ubr_trx_num; + ATOMIC_STORE((*trx)->ubr_id, g_ubr_trx_num); + g_ubr_mgr.trx_mgr_unit_id[i] = g_ubr_trx_num; (*trx)->close_state = UBR_CLOSE_FIRST; (*trx)->close_cnt = MAX_CLOSE_COUNT; ++g_ubr_mgr.trx_num; @@ -130,39 +277,219 @@ RETURN_CODE UBRingManager::AcquireUbrTrxFromMgr(UbrTrx **trx) { return UBRING_ERR; } -RETURN_CODE UBRingManager::ReleaseUbrTrxFromMgr(UbrTrx *trx) { - if (UNLIKELY(trx == nullptr)) { +RETURN_CODE UBRingManager::ReleaseUbrTrxFromMgr(UbrTrx *trx, + uint64_t expect_ubr_id) { + if (BAIDU_UNLIKELY(trx == nullptr)) { LOG(ERROR) << "Release trx failed, trx is null."; return UBRING_ERR; } - - trx->local_shm.addr = nullptr; - trx->ubr_tx.local_tx_event_q.addr = nullptr; - trx->ubr_tx.local_data_status_q.addr = nullptr; - trx->ubr_rx.local_rx_event_q.addr = nullptr; - trx->ubr_rx.remote_data_status_q.addr = nullptr; - if (UNLIKELY(g_ubr_mgr.trx_mgr == nullptr)) { + if (BAIDU_UNLIKELY(g_ubr_mgr.trx_mgr == nullptr)) { LOG(ERROR) << "Release trx failed, trx_mgr is null."; return UBRING_ERR; } - LOCK_GUARD(g_ubr_trx_mgr_mtx); + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); uint32_t idx = trx->trx_mgr_index; if (g_ubr_mgr.trx_mgr_unit_status[idx] == UBR_MGR_UNIT_FREE) { LOG(INFO) << "Release trx already freed, name=" << trx->local_shm.name; return UBRING_OK; } + if (BAIDU_UNLIKELY(g_ubr_mgr.trx_mgr_unit_id[idx] != expect_ubr_id)) { + // The slot was released and acquired again meanwhile; the stale + // caller must not touch the new occupant. + LOG(WARNING) << "Release stale trx refused, name=" << trx->local_shm.name; + return UBRING_OK; + } + if (g_ubr_mgr.trx_num == 0) { LOG(ERROR) << "Release trx failed, trx number is 0."; return UBRING_ERR; } + // Disarm the per-acquisition periodic timers before the slot becomes + // reusable: a surviving task would keep re-arming and firing against the + // slot's next occupant. Non-blocking delete only -- this path holds + // g_ubr_trx_mgr_mtx and the timer callbacks take the same lock, so + // UbrTimerDelAndWait would deadlock; it is not needed either, because + // every caller has already quiesced the per-trx callbacks before reaching + // here: the clear paths wait through UbrStopTrxTimer (or run on the single + // bthread timer thread) before they call ReleaseUbrTrxFromMgr, the force + // close waits in UbrTrxClose, and UbrMgrFini waits for all of them. + UbrTimerDel(&trx->close_timer); + UbrTimerDel(&trx->hb_timer); + + // Mutate the trx only after the generation check passed. + trx->local_shm.addr = nullptr; + trx->ubr_tx.local_tx_event_q.addr = nullptr; + trx->ubr_tx.local_data_status_q.addr = nullptr; + trx->ubr_rx.local_rx_event_q.addr = nullptr; + trx->ubr_rx.remote_data_status_q.addr = nullptr; g_ubr_mgr.trx_mgr_unit_status[idx] = UBR_MGR_UNIT_FREE; --g_ubr_mgr.trx_num; return UBRING_OK; } +UbrCleanupClaim UBRingManager::ClaimTrxCleanupForced( + uint32_t idx, uint64_t expect_ubr_id, UbrCleanupCtl** out_ctl) { + if (out_ctl != nullptr) { + *out_ctl = nullptr; + } + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + if (BAIDU_UNLIKELY(g_ubr_mgr.trx_mgr == nullptr || + g_ubr_mgr.trx_mgr_unit_status == nullptr || + g_ubr_mgr.trx_mgr_unit_id == nullptr || + g_ubr_mgr.trx_mgr_unit_ctl == nullptr || + idx >= g_ubr_mgr.trx_cap)) { + return UBR_CLEANUP_CLAIM_NOT_OURS; + } + // Same generation check as TryClaimTrxClose: a caller that snapshotted the + // slot before a release/reuse must not claim the new occupant's cleanup. + if (g_ubr_mgr.trx_mgr_unit_status[idx] != UBR_MGR_UNIT_USED || + g_ubr_mgr.trx_mgr_unit_id[idx] != expect_ubr_id) { + return UBR_CLEANUP_CLAIM_NOT_OURS; + } + UbrCleanupCtl* ctl = g_ubr_mgr.trx_mgr_unit_ctl[idx]; + if (ctl != nullptr) { + // An anchored ctl always belongs to the current generation: the + // publication checked it and the acquire retires the previous one. + // Keep the explicit check as a belt-and-braces guard. + if (BAIDU_UNLIKELY(ctl->ubr_id != expect_ubr_id)) { + return UBR_CLEANUP_CLAIM_NOT_OURS; + } + ctl->ref.fetch_add(1); // snapshot reference + if (out_ctl != nullptr) { + *out_ctl = ctl; + } + return UBR_CLEANUP_CLAIM_HAS_CTL; + } + // No cleanup was published: this call owns the forced cleanup, and + // refusing later publications keeps it the only owner. TryPublishUnitCleanupCtl + // checks the flag in this same critical section, so a publication either + // happened before (we would have seen its ctl above) or is refused. + g_ubr_mgr.trx_mgr[idx].cleanup_forced.store(true); + return UBR_CLEANUP_CLAIM_OWNED_NULL; +} + +bool UBRingManager::IsUbrTrxSlotUsed(uint32_t idx, uint64_t expect_ubr_id) { + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + if (BAIDU_UNLIKELY(g_ubr_mgr.trx_mgr_unit_id == nullptr || + g_ubr_mgr.trx_mgr_unit_status == nullptr || + idx >= g_ubr_mgr.trx_cap)) { + return false; + } + return g_ubr_mgr.trx_mgr_unit_status[idx] == UBR_MGR_UNIT_USED && + g_ubr_mgr.trx_mgr_unit_id[idx] == expect_ubr_id; +} + +bool UBRingManager::BeginPoolAccess() { + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + // Check every array the caller will walk, not just trx_mgr: UbrMgrInit can + // fail after allocating some of them, and that path calls UbrMgrFini with + // pool_ready == false. + if (BAIDU_UNLIKELY(g_ubr_mgr.shutting_down || + g_ubr_mgr.trx_mgr == nullptr || + g_ubr_mgr.trx_mgr_unit_status == nullptr || + g_ubr_mgr.trx_mgr_unit_id == nullptr || + g_ubr_mgr.trx_mgr_unit_ctl == nullptr)) { + return false; + } + ++g_ubr_mgr.active_pool_ops; + return true; +} + +void UBRingManager::FinishPoolAccess() { + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + if (g_ubr_mgr.active_pool_ops > 0) { + --g_ubr_mgr.active_pool_ops; + } +} + +bool UBRingManager::ArmTimersExclusive(const std::function& arm) { + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + if (BAIDU_UNLIKELY(g_ubr_mgr.shutting_down)) { + return false; + } + return arm(); +} + +RETURN_CODE UBRingManager::TryClaimTrxClose(uint32_t idx, uint64_t expect_ubr_id) { + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + if (BAIDU_UNLIKELY(g_ubr_mgr.trx_mgr == nullptr || + g_ubr_mgr.trx_mgr_unit_status == nullptr || + g_ubr_mgr.trx_mgr_unit_id == nullptr || + idx >= g_ubr_mgr.trx_cap)) { + return UBRING_ERR; + } + // A caller that snapshotted the slot before a release/reuse must not claim + // the close of the slot's new occupant: its generation check and the claim + // happen together here, under the lock the acquire/release paths also take. + if (g_ubr_mgr.trx_mgr_unit_status[idx] != UBR_MGR_UNIT_USED || + g_ubr_mgr.trx_mgr_unit_id[idx] != expect_ubr_id) { + return UBRING_ERR; + } + UbrTrx* trx = &g_ubr_mgr.trx_mgr[idx]; + // Validate before claiming, so a rejected close leaves the counter intact + // and the slot is not stuck in "closing" forever. + if (BAIDU_UNLIKELY(trx->ubr_tx.local_tx_event_q.addr == nullptr)) { + LOG(ERROR) << "Trx close failed, local_tx_event_q addr is NULL, trx local name=" + << trx->local_shm.name; + return UBRING_ERR; + } + int expected = MAX_CLOSE_COUNT; + if (!ATOMIC_COMPARE_EXCHANGE_STRONG(trx->close_cnt, expected, MAX_CLOSE_COUNT - 1)) { + return UBRING_REENTRY; + } + return UBRING_OK; +} + +bool UBRingManager::TryPublishUnitCleanupCtl(uint32_t idx, + uint64_t expect_ubr_id, + UbrCleanupCtl *ctl) { + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + if (BAIDU_UNLIKELY(g_ubr_mgr.shutting_down || + g_ubr_mgr.trx_mgr == nullptr || + g_ubr_mgr.trx_mgr_unit_ctl == nullptr || + g_ubr_mgr.trx_mgr_unit_status == nullptr || + g_ubr_mgr.trx_mgr_unit_id == nullptr || + idx >= g_ubr_mgr.trx_cap || + g_ubr_mgr.trx_mgr_unit_status[idx] != UBR_MGR_UNIT_USED || + g_ubr_mgr.trx_mgr_unit_id[idx] != expect_ubr_id || + g_ubr_mgr.trx_mgr[idx].cleanup_forced.load() || + g_ubr_mgr.trx_mgr_unit_ctl[idx] != nullptr)) { + return false; // shutting down / released / reused / forced / already anchored + } + // Publish on the trx and anchor it in the pool slot inside one critical + // section: a force close that snapshots the slot (also under this lock) + // can then never observe the trx-side publication without its anchor, nor + // claim the no-ctl cleanup path for a cleanup that is being scheduled. + UbrCleanupCtl* expected = nullptr; + if (!g_ubr_mgr.trx_mgr[idx].cleanup_ctl.compare_exchange_strong(expected, ctl)) { + return false; // another schedule won + } + ctl->ref.fetch_add(1); // manager anchor reference + g_ubr_mgr.trx_mgr_unit_ctl[idx] = ctl; + return true; +} + +bool UBRingManager::DetachUnitCleanupCtl(uint32_t idx, UbrCleanupCtl *ctl) { + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + if (BAIDU_UNLIKELY(g_ubr_mgr.trx_mgr == nullptr || + g_ubr_mgr.trx_mgr_unit_ctl == nullptr || + idx >= g_ubr_mgr.trx_cap || + g_ubr_mgr.trx_mgr_unit_ctl[idx] != ctl)) { + return false; + } + g_ubr_mgr.trx_mgr_unit_ctl[idx] = nullptr; + // Clear the trx-side publication in the same critical section, and only + // while it still belongs to `ctl': a slot reused meanwhile may already + // carry the new occupant's control object. + UbrCleanupCtl* published = ctl; + g_ubr_mgr.trx_mgr[idx].cleanup_ctl.compare_exchange_strong(published, nullptr); + ctl->ReleaseRef(); // manager reference + return true; +} + void UBRingManager::LinkInfoInit(void) { size_t link_info_mgr_size = FLAGS_ubr_max_managed_num * sizeof(UbrLinkInfo); @@ -189,7 +516,7 @@ void UBRingManager::LinkInfoFini(void) { return; } { - LOCK_GUARD(g_link_info_mgr_mtx); + BAIDU_SCOPED_LOCK(g_link_info_mgr_mtx); FREE_PTR(g_link_info_mgr.all_link_info); FREE_PTR(g_link_info_mgr.link_mgr_unit_status); } @@ -235,30 +562,54 @@ void UBRingManager::ReleaseLinkInfoFromMgr(UbrTrx *trx) { int32_t UBRingManager::UbEventCallback(const char *shm_name) { - if (UNLIKELY(shm_name == nullptr)) { + if (BAIDU_UNLIKELY(shm_name == nullptr)) { LOG(ERROR) << "Ub event callback failed, shm name is null."; return UBRING_ERR; } - if (UNLIKELY(g_ubr_mgr.trx_mgr == nullptr)) { - LOG(ERROR) << "Ub event callback failed, trx mgr is null."; + // This callback runs on an SDK thread with no timer or cleanup reference + // keeping the pool alive, so register the access first: it is refused once + // UbrMgrFini started, and UbrMgrFini waits for the accepted accesses before + // it frees the pool. The UbrTrx* below is therefore valid until + // FinishPoolAccess, even though the lookup lock is released in between. + if (!BeginPoolAccess()) { + LOG(WARNING) << "Ub event callback skipped, ubr manager is shutting down. shm_name=" + << shm_name; return UBRING_ERR; } - LOG(INFO) << "Ub event callback is processing. shm_name=" << shm_name; - for (uint32_t i = 0; i < g_ubr_mgr.trx_cap; ++i) { - if (g_ubr_mgr.trx_mgr_unit_status[i] == UBR_MGR_UNIT_FREE) { - continue; - } - - if (strcmp(g_ubr_mgr.trx_mgr[i].local_shm.name, shm_name) == 0 || // the failed link is this trx's local shm - strcmp(g_ubr_mgr.trx_mgr[i].remote_shm.name, shm_name) == 0) { // the failed link is this trx's remote shm - ++g_ub_event_cnt; - int fd = (int)g_ubr_mgr.trx_mgr[i].local_shm.fd; - LOG(WARNING) << "Ub event callback, the fd of the faulty link is " << fd; - return UBRing::UbrPassiveClearTrx(&g_ubr_mgr.trx_mgr[i], fd, UBR_UB_EVENT); + // Look the faulty link up under the manager lock: the pool walk reads the + // slot status and the shm names, both of which a concurrent acquire or + // release rewrites. The generation is snapshotted in the same critical + // section, and the close claim below re-validates it, so a slot released + // and reused after this lookup cannot be closed by mistake. + uint32_t idx = 0; + uint64_t expect_ubr_id = 0; + int fd = -1; + bool found = false; + { + BAIDU_SCOPED_LOCK(g_ubr_trx_mgr_mtx); + for (uint32_t i = 0; i < g_ubr_mgr.trx_cap; ++i) { + if (g_ubr_mgr.trx_mgr_unit_status[i] == UBR_MGR_UNIT_FREE) { + continue; + } + if (strcmp(g_ubr_mgr.trx_mgr[i].local_shm.name, shm_name) == 0 || // the failed link is this trx's local shm + strcmp(g_ubr_mgr.trx_mgr[i].remote_shm.name, shm_name) == 0) { // the failed link is this trx's remote shm + idx = i; + expect_ubr_id = ATOMIC_LOAD(g_ubr_mgr.trx_mgr[i].ubr_id); + fd = (int)g_ubr_mgr.trx_mgr[i].local_shm.fd; + found = true; + break; + } } } - return UBRING_ERR; + int32_t rc = UBRING_ERR; + if (found) { + ++g_ub_event_cnt; + LOG(WARNING) << "Ub event callback, the fd of the faulty link is " << fd; + rc = UBRing::UbrPassiveClearTrx(&g_ubr_mgr.trx_mgr[idx], expect_ubr_id); + } + FinishPoolAccess(); + return rc; } } } diff --git a/src/brpc/ubshm/ub_ring_manager.h b/src/brpc/ubshm/ub_ring_manager.h index e2afa324c7..711c06e339 100644 --- a/src/brpc/ubshm/ub_ring_manager.h +++ b/src/brpc/ubshm/ub_ring_manager.h @@ -18,6 +18,8 @@ #ifndef BRPC_UB_RING_MANAGER_H #define BRPC_UB_RING_MANAGER_H +#include + #include "brpc/ubshm/ubr_trx.h" #include "brpc/ubshm/shm/shm_def.h" #include "brpc/ubshm/common/common.h" @@ -29,11 +31,35 @@ typedef enum { UBR_MGR_UNIT_USED = 1 } UbrMgrUnitStatus; +// Result of UBRingManager::ClaimTrxCleanupForced. +typedef enum { + // The slot is not (or no longer) used by the requested generation: the + // caller must not touch it and must not run the cleanup. + UBR_CLEANUP_CLAIM_NOT_OURS = 0, + // A delayed cleanup is published for that generation and was handed back + // in *out_ctl with a snapshot reference the caller must release. + UBR_CLEANUP_CLAIM_HAS_CTL = 1, + // No delayed cleanup existed; the forced claim took the cleanup ownership + // and later publications for that generation are refused. + UBR_CLEANUP_CLAIM_OWNED_NULL = 2 +} UbrCleanupClaim; + typedef struct TagUbrMgr { uint32_t trx_num; uint32_t trx_cap; + // Set under g_ubr_trx_mgr_mtx once UbrMgrFini started: no pool slot may be + // acquired afterwards, so the shutdown path only has to wait for the + // connections that were already in the pool when it took the flag. + bool shutting_down; + // Callback-driven operations that read the pool without owning a timer or + // cleanup reference (the faulty-shm event lookup). Guarded by + // g_ubr_trx_mgr_mtx; UbrMgrFini waits for it to drop to zero before it + // frees the pool. + uint32_t active_pool_ops; UbrTrx *trx_mgr; UbrMgrUnitStatus *trx_mgr_unit_status; + uint64_t *trx_mgr_unit_id; + UbrCleanupCtl **trx_mgr_unit_ctl; } UbrMgr; typedef struct TagUbrLinkInfo { @@ -59,11 +85,97 @@ class UBRingManager { static RETURN_CODE UbrMgrInit(); + // Tear the pool down. Contract: no connection may be in setup or teardown + // while this runs. The shutdown barrier below only covers the paths that + // register themselves (pool acquisitions, timer arming, cleanup-control + // publication, and the faulty-shm callback through BeginPoolAccess); + // UbrTrxClose / ReleaseUbrTrxFromMgr and the rest of a connection setup + // keep touching UbrTrx and would race the FREE_PTR at the end. Today this + // is only reachable from the init-failure path and from tests, but a real + // graceful-shutdown caller must drain the connections first (or extend the + // barrier to those paths). + // + // This is terminal: the shutdown flag stays set and the pool stays freed, + // so the manager only becomes usable again through a new UbrMgrInit (which + // clears the flag and reallocates the pool) -- not by calling Fini twice. static void UbrMgrFini(); static RETURN_CODE AcquireUbrTrxFromMgr(UbrTrx **trx); - static RETURN_CODE ReleaseUbrTrxFromMgr(UbrTrx *trx); + // Release the pool slot of `trx'. `expect_ubr_id' must be snapshotted + // from trx->ubr_id before the caller started releasing the trx: a + // release racing a reuse of the same slot is refused. + static RETURN_CODE ReleaseUbrTrxFromMgr(UbrTrx *trx, + uint64_t expect_ubr_id); + + // Snapshot the delayed cleanup control object of a pool slot and, when + // there is none, claim the cleanup of `expect_ubr_id' for the force close + // that will run it. Both the snapshot and the claim happen under the + // manager lock, i.e. in the same critical section as + // TryPublishUnitCleanupCtl, so the two are totally ordered: either the + // publication ran first (UBR_CLEANUP_CLAIM_HAS_CTL, and the existing + // ctl->state arbitration decides who runs the cleanup) or the claim ran + // first (UBR_CLEANUP_CLAIM_OWNED_NULL, and every later publication for + // this generation is refused through trx->cleanup_forced). Splitting the + // two steps -- a snapshot, then an out-of-lock re-check that no ctl + // appeared -- leaves a window in which a concurrent SDK-fault callback + // publishes a new cleanup after an empty snapshot and both paths run the + // cleanup. `*out_ctl' is only written on UBR_CLEANUP_CLAIM_HAS_CTL, and + // carries a snapshot reference the caller must ReleaseRef. + static UbrCleanupClaim ClaimTrxCleanupForced(uint32_t idx, + uint64_t expect_ubr_id, + UbrCleanupCtl** out_ctl); + + // Under the manager lock, confirm the pool slot is still used by the + // given generation (not released or reused meanwhile). + static bool IsUbrTrxSlotUsed(uint32_t idx, uint64_t expect_ubr_id); + + // Atomically claim the close of the pool slot `idx' for the generation + // `expect_ubr_id': under the manager lock it validates that the slot is + // still used by that generation -- so a snapshot taken before a + // release/reuse cannot claim the next occupant's close -- and then moves + // the close counter from MAX_CLOSE_COUNT to MAX_CLOSE_COUNT - 1 so no + // other path can claim the same close. Returns UBRING_REENTRY when the + // same generation is already closing, UBRING_ERR when the caller must not + // touch the slot (released, reused, or not fully initialized). + static RETURN_CODE TryClaimTrxClose(uint32_t idx, uint64_t expect_ubr_id); + + // Atomically (under the manager lock) publish `ctl' on the pool slot and + // anchor it there, but only while the slot is still used by the given + // generation and carries no other cleanup control object. Both the + // trx-side field and the manager anchor are set in the same critical + // section, so a concurrent force close never observes one without the + // other. On success the ctl gains the manager anchor reference (released + // when the anchor is detached or the slot is retired); returns false -- + // leaving the slot and the reference counts untouched -- when the trx was + // released, reused, a force close already claimed the cleanup, or a + // cleanup is already anchored. + static bool TryPublishUnitCleanupCtl(uint32_t idx, uint64_t expect_ubr_id, + UbrCleanupCtl *ctl); + + // Detach `ctl' from the pool slot if it is still anchored there, clearing + // the trx-side field in the same critical section (and only while it still + // belongs to `ctl'). Returns true when the detach happened (the manager + // reference was released). + static bool DetachUnitCleanupCtl(uint32_t idx, UbrCleanupCtl *ctl); + + // Register / release a callback-driven pool access that does not own a + // timer or cleanup reference (currently the faulty-shm event lookup). + // BeginPoolAccess fails once UbrMgrFini started, and UbrMgrFini waits for + // the accepted accesses to finish before it frees the pool, so the caller + // may keep using a UbrTrx* it obtained while the access is open. Every + // successful BeginPoolAccess must be paired with FinishPoolAccess, which + // is safe to call after UbrMgrFini returned. + static bool BeginPoolAccess(); + static void FinishPoolAccess(); + + // Run `arm' with the manager lock held, and only while the manager is not + // shutting down: arming a per-trx timer and taking the shutdown flag are + // then serialized, so once UbrMgrFini set the flag no new timer can appear + // behind its wait loop. `arm' must not block on a timer callback (it may + // only start timers); it returns whether everything was armed. Returns + // false when the manager is shutting down, in which case `arm' never ran. + static bool ArmTimersExclusive(const std::function& arm); static void LinkInfoInit(void); static void LinkInfoFini(void); diff --git a/src/brpc/ubshm/ubr_trx.h b/src/brpc/ubshm/ubr_trx.h index 4111b8ee87..0c30ea8fb7 100644 --- a/src/brpc/ubshm/ubr_trx.h +++ b/src/brpc/ubshm/ubr_trx.h @@ -25,6 +25,7 @@ #include "brpc/ubshm/common/thread_lock.h" #include "brpc/ubshm/ubr_data_format.h" #include "brpc/ubshm/ubr_msg.h" +#include "brpc/ubshm/timer/timer_mgr.h" /* +----------------------------------------------------------------------------+ │ UbrTrx shm │ @@ -58,17 +59,22 @@ typedef enum { UBR_STATE_CLOSED } EventQState; -typedef enum { - UBR_SEND_CLOSE, - UBR_CALL_BACK_CLOSE -} UbrCloseType; - typedef enum { UBR_CLOSE_FIRST, UBR_CLOSE_SECOND, UBR_CLOSE_END } UbrCloseCount; +// Ownership of the final delayed cleanup of a trx. Exactly one side (the +// delayed-clear callback or a force close) may run it. The state lives in +// a per-acquisition heap control object (UbrCleanupCtl) anchored by the +// trx manager, so it survives the pool slot reuse that wipes UbrTrx. +typedef enum { + UBR_CLEANUP_PENDING = 1, // cleanup timer scheduled + UBR_CLEANUP_RUNNING = 2, // cleanup owned, in progress + UBR_CLEANUP_DONE = 3 // cleanup finished +} UbrCleanupState; + typedef enum { UDP_TRX, TCP_TRX, @@ -105,11 +111,11 @@ typedef struct TagUbrTx { UbrAddrInfo remote_rx_event_q; UbrAddrInfo local_data_status_q; UbrAddrInfo local_tx_event_q; - uint64_t out_io_id; + AtomicUintFast64 out_io_id; uint32_t write_pos; uint32_t capacity; UbrMsgFormat local_msg_space; - uint32_t hb_retry_cnt; + int32_t hb_retry_cnt; uint32_t ep_last_cap; volatile EventQState trx_state; } UbrTx; @@ -119,7 +125,7 @@ typedef struct TagUbrRx { UbrAddrInfo local_rx_event_q; UbrAddrInfo remote_data_status_q; UbrAddrInfo remote_tx_event_q; - uint64_t in_io_id; + AtomicUintFast64 in_io_id; uint32_t read_pos; uint32_t capacity; uint32_t deal_msg_num; @@ -129,18 +135,57 @@ typedef struct TagUbrRx { volatile EventQState trx_state; } UbrRx; +struct TagUbrTrx; + +// Control object of one delayed cleanup, created when the cleanup is +// scheduled and destroyed when the owning pool slot is acquired again. +// It is anchored by the trx manager (not by the reusable UbrTrx memory), +// so the ownership claim and the completion signal stay valid across the +// release/reuse of the trx that owns the cleanup. +struct UbrCleanupCtl { + TagUbrTrx* trx; // immutable + uint64_t ubr_id; // immutable generation + AtomicInt state; // UbrCleanupState + butil::atomic timer; // delayed clear timer + // References: timer/callback (held from a successful schedule until + // the callback fully returned, or released by whoever cancels the + // timer before it fires) + starter (until the schedule call + // completes) + manager anchor (taken when the ctl is anchored to the + // pool slot, released when the anchor is detached or retired). + AtomicInt ref; + + void ReleaseRef() { + if (ref.fetch_sub(1) == 1) { + delete this; + } + } +}; + typedef struct TagUbrTrx { UbrTx ubr_tx; UbrRx ubr_rx; - uint64_t ubr_id; + AtomicUintFast64 ubr_id; uint32_t trx_mgr_index; UbrTrxType type; UbrDataFormat data_format; SHM local_shm; SHM remote_shm; - int timer_fd; - int hb_timer_fd; - int clear_timer_fd; + butil::atomic close_timer; + butil::atomic hb_timer; + butil::atomic cleanup_ctl; + // True once a force close claimed the cleanup of this acquisition while no + // delayed cleanup was published (see + // UBRingManager::ClaimTrxCleanupForced). The forced claim and the + // publication of a delayed cleanup are serialized by the manager lock, so + // a later publication sees the flag and refuses: only one of the two ever + // owns the cleanup. Reset by AcquireUbrTrxFromMgr together with the other + // per-acquisition fields; a stale true on a FREE slot is harmless because + // a publication is already refused while the slot is not USED. + butil::atomic cleanup_forced; + // Last io ids seen by the close-check timer, used to reset its + // back-off polling interval when the link has traffic. + uint64_t close_chk_in_io_id; + uint64_t close_chk_out_io_id; AtomicInt close_cnt; AtomicInt close_state; } UbrTrx; @@ -155,11 +200,6 @@ typedef struct TagUbrLinkLock { FileLock* file_lock; } UbrLinkLock; -typedef enum { - UBR_UB_EVENT, - UBR_HEARTBEAT, -}PASSIVE_DISC_TYPE; - } } #endif //BRPC_UBR_TRX_H diff --git a/test/brpc_naming_service_unittest.cpp b/test/brpc_naming_service_unittest.cpp index 2c5ad76d94..a01e033f61 100644 --- a/test/brpc_naming_service_unittest.cpp +++ b/test/brpc_naming_service_unittest.cpp @@ -289,6 +289,18 @@ TEST(NamingServiceTest, remotefile) { } } +TEST(NamingServiceTest, remotefile_invalid_url) { + const char* service_names[] = { + "http://", "bns://", "http:////" + }; + for (const char* service_name : service_names) { + brpc::policy::RemoteFileNamingService rfns; + std::vector servers; + EXPECT_EQ(-1, rfns.GetServers(service_name, &servers)); + EXPECT_TRUE(servers.empty()); + } +} + // Build a JSON array nested `depth' levels deep, e.g. depth=2 -> "[[]]". static std::string MakeDeeplyNestedJson(int depth) { return std::string(depth, '[') + std::string(depth, ']'); diff --git a/test/brpc_ubring_unittest.cpp b/test/brpc_ubring_unittest.cpp index c9b9c91db1..2f92c8d725 100644 --- a/test/brpc_ubring_unittest.cpp +++ b/test/brpc_ubring_unittest.cpp @@ -24,11 +24,13 @@ #include #include #include "butil/macros.h" +#include "butil/atomicops.h" #include "butil/sys_byteorder.h" #include "brpc/socket.h" #if BRPC_WITH_UBRING #include "brpc/ubshm/common/common.h" +#include "brpc/ubshm/ub_cleanup_worker.h" #include "brpc/ubshm/ub_endpoint.h" #include "brpc/ubshm/shm/shm_def.h" #include "brpc/ubshm/shm/shm_mgr.h" @@ -43,9 +45,11 @@ DECLARE_int32(ub_disconnect_timeout_s); DECLARE_int32(ub_connect_timeout_s); DECLARE_int32(ub_hb_timer_interval_s); DECLARE_int32(ub_event_queue_timer_interval_us); +DECLARE_int32(ub_event_queue_timer_interval_max_us); DECLARE_int32(ub_flying_io_timeout_s); extern bool g_skip_ub_init; +extern butil::atomic g_ub_test_worker_start_rc; } // namespace ubring } // namespace brpc @@ -364,8 +368,10 @@ TEST(IpcV2FormatPreparationTest, prepares_layout_without_activation) { EXPECT_EQ(0u, trx.ubr_rx.ipc_v2_read_offset); EXPECT_EQ(UBR_STATE_NONE, trx.ubr_tx.trx_state); EXPECT_EQ(UBR_STATE_NONE, trx.ubr_rx.trx_state); - EXPECT_EQ(0, trx.timer_fd); - EXPECT_EQ(0, trx.hb_timer_fd); + // The close/heartbeat timers must stay unarmed (the pre-facade fields + // timer_fd/hb_timer_fd became the close_timer/hb_timer slots). + EXPECT_EQ(nullptr, trx.close_timer.load()); + EXPECT_EQ(nullptr, trx.hb_timer.load()); EXPECT_EQ(91u, local_status.tail); EXPECT_EQ(1u, __atomic_load_n(&remote_status.tail, __ATOMIC_ACQUIRE)); EXPECT_EQ(111u, local_status.timeout); @@ -784,6 +790,7 @@ TEST(UBRingConfigurationTest, time_flags_include_units_and_expected_defaults) { {"ub_connect_timeout_s", "_s", "seconds", "1"}, {"ub_hb_timer_interval_s", "_s", "seconds", "5"}, {"ub_event_queue_timer_interval_us", "_us", "microseconds", "100"}, + {"ub_event_queue_timer_interval_max_us", "_us", "microseconds", "10000"}, {"ub_flying_io_timeout_s", "_s", "seconds", "5"}, }; @@ -803,6 +810,7 @@ TEST(UBRingConfigurationTest, time_flags_include_units_and_expected_defaults) { EXPECT_EQ(1, brpc::ubring::FLAGS_ub_connect_timeout_s); EXPECT_EQ(5, brpc::ubring::FLAGS_ub_hb_timer_interval_s); EXPECT_EQ(100, brpc::ubring::FLAGS_ub_event_queue_timer_interval_us); + EXPECT_EQ(10000, brpc::ubring::FLAGS_ub_event_queue_timer_interval_max_us); EXPECT_EQ(5, brpc::ubring::FLAGS_ub_flying_io_timeout_s); EXPECT_EQ(100U * USEC_TO_NSEC, static_cast( @@ -810,6 +818,58 @@ TEST(UBRingConfigurationTest, time_flags_include_units_and_expected_defaults) { USEC_TO_NSEC); } +// --------------------------------------------------------------------------- +// UbrCleanupWorker: a failed lazy start must be retried, not latched. + +namespace { +butil::atomic g_ub_test_cleanup_runs(0); + +void UbTestCleanupStep(brpc::ubring::ShmList*) { + g_ub_test_cleanup_runs.fetch_add(1); +} +} // namespace + +// The worker is a process-wide singleton started lazily by the first post and +// never stopped, so its lazy start can only be observed before any other test +// posted a cleanup job. This suite is therefore declared ahead of +// UBShmEndpointTest, whose teardown paths post cleanups. Should a shuffled +// run still find the worker started, the probe below detects that and skips +// instead of asserting on a start that is never attempted. +TEST(UbrCleanupWorkerTest, start_failure_is_retried_not_latched) { + using brpc::ubring::UbrCleanupWorker; + brpc::ubring::ShmList shm_list; + memset(&shm_list, 0, sizeof(shm_list)); + g_ub_test_cleanup_runs.store(0); + + // Fail the lazy start exactly like a transient bthread creation failure + // (e.g. ENOMEM) would. + brpc::ubring::g_ub_test_worker_start_rc.store(-1); + + // The first post doubles as the probe: a post that succeeds despite the + // injection means the worker was already started by an earlier test and + // no start -- the code under test -- is attempted anymore. + if (UbrCleanupWorker::PostShmDrain(&shm_list, UbTestCleanupStep)) { + brpc::ubring::g_ub_test_worker_start_rc.store(0); + UbrCleanupWorker::DrainAndWait(); + GTEST_SKIP() << "the cleanup worker was already started by an " + "earlier test; the lazy start cannot be exercised"; + } + + // While the start keeps failing, every post fails and fails only itself: + // no job is queued, so the caller must run it inline. + EXPECT_FALSE(UbrCleanupWorker::PostShmDrain(&shm_list, UbTestCleanupStep)); + EXPECT_FALSE(UbrCleanupWorker::PostShmDrain(&shm_list, UbTestCleanupStep)); + EXPECT_EQ(0, g_ub_test_cleanup_runs.load()); + + // The failure must not latch: clear the injection and post again. The + // start is retried, the worker comes up, the job runs exactly once and + // DrainAndWait observes quiescence. + brpc::ubring::g_ub_test_worker_start_rc.store(0); + EXPECT_TRUE(UbrCleanupWorker::PostShmDrain(&shm_list, UbTestCleanupStep)); + UbrCleanupWorker::DrainAndWait(); + EXPECT_EQ(1, g_ub_test_cleanup_runs.load()); +} + namespace brpc { namespace ubring { class UBShmEndpointTest : public ::testing::Test {