zchuango commented on code in PR #3509:
URL: https://github.com/apache/brpc/pull/3509#discussion_r4120001441
##########
src/brpc/ubshm/shm/shm_ubs.cpp:
##########
@@ -413,55 +414,58 @@ static void DeleteShmToList(ShmList* shm_list)
void *UbsShmCallback(void* args)
{
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.";
+ // Drain one node per fire and keep the SDK calls outside the lock, so
+ // a slow daemon cannot stall the timer thread for the whole list.
+ SHM shm;
+ {
+ BAIDU_SCOPED_LOCK(shm_list->shm_lock);
Review Comment:
brpc provides a std::lock_guard<pthread_mutex_t> specialization in
butil/scoped_lock.h, which calls pthread_mutex_lock/unlock. Therefore
BAIDU_SCOPED_LOCK supports this mutex type. The Linux and macOS builds also
pass on this commit.
##########
src/brpc/ubshm/ub_ring_manager.cpp:
##########
@@ -68,57 +77,123 @@ 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);
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() {
+ // 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);
Review Comment:
brpc provides a std::lock_guard<pthread_mutex_t> specialization in
butil/scoped_lock.h, which calls pthread_mutex_lock/unlock. Therefore
BAIDU_SCOPED_LOCK supports this mutex type. The Linux and macOS builds also
pass on this commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
