Skip to content

fix(cache): make ICacheStore release lifecycle atomic - #1731

Closed
xiaoyang-hhh wants to merge 1 commit into
alibaba:mainfrom
xiaoyang-hhh:fix/cache-store-release-lifecycle
Closed

xiaoyang-hhh wants to merge 1 commit into
alibaba:mainfrom
xiaoyang-hhh:fix/cache-store-release-lifecycle

Conversation

@xiaoyang-hhh

Copy link
Copy Markdown
Collaborator

Summary

  • keep the detach flag and external reference count in one atomic state word so the final release action uses a consistent state
  • detach before dropping the caller reference, and preserve the ObjectCache reference handoff when reopening an idle store
  • avoid holding the store spinlock across pool release and add N3 plus concurrent open/release/detach regression coverage

Tests

  • CachePool.concurrent_open_release_stress: 50 consecutive runs passed
  • CachePool.release_detach_vs_same_vcpu_reader_no_hang: 10 consecutive runs passed
  • full cache_test: 24/24 passed

ICacheStore::release() held mt_ -- a photon::spinlock, so a pure busy-wait
that never yields -- across pool_->store_release(). That call ends in
ExpireContainerBase::expire(), whose delete_all() destroys other stores:
~FileCacheStore closes files and takes the pool's m_lock_, so it can block
far longer than the store cache TTL. Once the deadline passes another
thread's expire() legitimately frees this store, and the original releaser
then unwinds out of SCOPED_LOCK(mt_) and writes one byte to freed memory
in spinlock::unlock().

The same window is worse than a stray write. The object is destroyed while
mt_ is still held and its destructor never unlocks it, so any thread that
later reaches release() -- or tryget_size(), which shares mt_ -- spins on
that lock forever. Observed live under a 4G disk cache: 15 threads pegged
in spinlock::lock(), 420% CPU, 75 FUSE handles stuck, no progress for 23
hours. The unfixed build hung after 6.5h; the fixed one ran 25h with 4.2x
the store-event volume and no report.

Replace the mt_-protected lifecycle with an atomic detached_ state. The
unique detacher removes the store from ObjectCache before dropping its own
reference, and the final reference decrement uses acq_rel. Attached stores
return their cache-held reference to ObjectCache; detached stores are
deleted by their last external reference. Open handles therefore stay alive
across detach and each store is destroyed exactly once without running
blocking pool operations under mt_. Drop the unused need_detach_ flag.

Keep the deterministic release_detach_vs_same_vcpu_reader_no_hang
regression test, which reproduces the same-vCPU spinlock starvation through
the real FileCachePool/FileCacheStore stack.
@xiaoyang-hhh
xiaoyang-hhh marked this pull request as draft November 6, 2026 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant