zchuango commented on code in PR #3509:
URL: https://github.com/apache/brpc/pull/3509#discussion_r4120099720


##########
src/brpc/ubshm/ub_ring.cpp:
##########
@@ -47,21 +56,130 @@ 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;
 }
 
+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, void* (*cb)(void*),
+                                         void (*work)(UbrTrx*, uint64_t)) {
+    if (BAIDU_UNLIKELY(trx == nullptr || trx->local_shm.addr == nullptr)) {
+        return UBRING_OK;                    // released trx, stale event
+    }
+    if (trx->cleanup_ctl.load() != nullptr) {
+        return UBRING_OK;                    // cleanup already scheduled

Review Comment:
   We’re checking the callback and trx slot lifetime and will follow up with 
either a fix or the invariant that rules out this race.



-- 
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]

Reply via email to