Repository navigation
Conversation
…eturns UbrTimerDel wins the handle-slot exchange and reports success for a one-shot timer, but a wrapper that the timer thread already dispatched may still be between its dispatch and its own (losing) slot exchange. The caller of UbrTimerDel treats return 0 as "the callback will not run" and frees the object that stores the slot -- UbrCleanupCtl on the delayed-clear path -- so that late wrapper access is a use-after-free. Track the slot retirement on the task: the one-shot wrapper sets slot_retired right after its slot CAS and before running the user callback, and UbrTimerDel waits for it when bthread_timer_del reports the task as dispatched. Callers that win the cancellation get the same guarantee from the delete itself. The wait only spins on a task-internal flag, so it is safe to do under the trx manager lock that the force-close, pool-acquire and UbrMgrFini callers hold, and it is limited to the one-shot path (a periodic task's slot storage is pool memory that the non-blocking callers never free).
The force close used to snapshot the pool slot's cleanup control object under the manager lock and then re-verify "still no ctl" out of the lock. A concurrent SDK-fault callback could publish a delayed cleanup for the same generation in between, so both the force close and the delayed-clear callback would run the cleanup of the same shared memory (double unmap / stale event queues). Replace the two steps with ClaimTrxCleanupForced, which snapshots the ctl and, when there is none, claims the cleanup for the generation inside one critical section, and make TryPublishUnitCleanupCtl refuse to publish once that claim happened (new per-acquisition trx->cleanup_forced flag, reset by the next AcquireUbrTrxFromMgr). Publication and claim are now totally ordered by the manager lock, so exactly one of them owns the cleanup. An existing publication keeps the current ctl->state arbitration untouched.
Timers are bthread based, so every callback in the process runs on one global timer thread. UbsShmCallback and the delayed-clear callbacks still called the UBS SDK there (ubsmem_shmem_unmap/deallocate), and those calls can wait on the SDK daemon with no latency bound, so a slow or failed shm release delayed every other timer in the process -- RPC timeouts, backup requests, bthread_usleep wakeups and ubring's own heartbeat/close checks. Add a lazily started cleanup worker bthread with an MPSC job queue. The timer callbacks now only arbitrate cleanup ownership and post a job (shared-memory drain or trx cleanup); the worker runs the blocking part, and takes over the timer/callback reference of the cleanup control object. Callers run the work inline only when the worker cannot be started. The drain keeps the previous retry semantics (the node stays in the list on unmap failure and the periodic timer retries it) and at most one drain step is in flight, and DestroyShmTimer drains the worker before it frees the list and finalizes the SDK. UbrMgrFini already waits for the control-object references, so it needs no change.
Post() was not serialized with the lazy start of the worker bthread. Two posters could decide "the worker is started" and "enqueue the job" independently: one thread won the start CAS and was about to see bthread_start_background fail, while another read g_worker_started == true and enqueued a job. Since g_worker_unavailable is terminal and no worker ever runs, that job is never dequeued -- its UbrCleanupCtl reference leaks, the pool slot is never released, and g_cleanup_inflight never returns to zero, so every later UbrCleanupWorker::DrainAndWait (DestroyShmTimer on a teardown path) hangs forever. Take a small mutex in Post() around the unavailable check, the start attempt and the enqueue, so a job can only ever be queued behind a worker that really started. Posting only happens on cleanup paths, the worker never takes the mutex, and a failed start now returns false (callers keep running the work inline) without having enqueued anything.
The UbrCleanupClaim members were the only constants in ub_ring_manager.h without the module's UBR_ prefix, next to UBR_MGR_UNIT_* and UBR_CLEANUP_*. Rename them to UBR_CLEANUP_CLAIM_NOT_OURS / _HAS_CTL / _OWNED_NULL.
…ract The Timer Management sections still implied all cleanup work runs on the timer thread. Document that the delayed-clear and pending-unmap callbacks only arbitrate ownership and post a job to the cleanup worker, and that a one-shot UbrTimerDel returning 0 now also guarantees the facade is done with the storage the handle slot lives in, so the caller may free the object holding it (UbrCleanupCtl on the delayed-clear path). Also refresh the ordering comment above UbrTrxCloseCallback: with the cleanup body on the worker, the guarantee rests on the generation gate the worker re-applies under the manager lock (IsUbrTrxSlotUsed) plus the per-trx UbrStopTrxTimer/UbrTimerDelAndWait waits, not on the timer thread serializing the cleanup itself.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.