Skip to content

[server][dvc] Fix use-after-free in RMD block cache metrics callbacks - #2742

Merged
sushantmane merged 2 commits into
linkedin:mainfrom
sushantmane:sumane/fix-rmd-cache-use-after-free
Apr 15, 2026
Merged

[server][dvc] Fix use-after-free in RMD block cache metrics callbacks#2742
sushantmane merged 2 commits into
linkedin:mainfrom
sushantmane:sumane/fix-rmd-cache-use-after-free

Conversation

@sushantmane

@sushantmane sushantmane commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix JVM crashes (SIGSEGV) caused by use-after-free in RMD block cache metrics callbacks.

The RMD block cache async gauge callbacks captured rmdCache directly
via method references (rmdCache::getUsage, rmdCache::getPinnedUsage)
with no lifecycle guard. When RocksDBStorageEngineFactory.close() frees
the native Cache object, the OTel/Tehuti callbacks still fire on the
next 60-second collection cycle, dereferencing the stale JNI handle and
causing a SIGSEGV.

This caused 21+ JVM crashes across EI and prod over 6 days. 82% of
crashes were on the collectionWindowExecutor-1 (OTel) or
Async_Gauge_Executor (Tehuti) metrics threads. The dominant crash
signature was a classic C++ vtable use-after-free: the freed Cache
memory was reused by the Java heap, so virtual method dispatch jumped
to garbage addresses containing Java class metadata.

The existing partition-level metrics (block-cache-capacity, etc.) are
safe because they go through getRocksDBStatValue() which is guarded
by synchronized(hostedRocksDBPartitions) and readCloseRWLock. The
RMD cache callbacks bypassed all of these guards.

Fix: Store the Cache reference in a volatile field. Callbacks read it
into a local variable before null-checking — the local pins the reference
on the stack so it cannot become null between check and JNI call (no
TOCTOU race, no locks needed). closeRMDBlockCache() nulls the volatile
reference, and is called from RocksDBStorageEngineFactory.close() BEFORE
sharedRMDCache.close() to ensure callbacks see null before native memory
is freed.

Testing Done

  • Verified all existing RocksDB tests pass
  • Code review of close() ordering in RocksDBStorageEngineFactory
  • Confirmed volatile + local variable pattern eliminates TOCTOU race
    without requiring locks (same pattern used in JDK concurrent utilities)

The RMD block cache async gauge callbacks captured `rmdCache` directly
via method references (`rmdCache::getUsage`, `rmdCache::getPinnedUsage`)
with no lifecycle guard. When `RocksDBStorageEngineFactory.close()` frees
the native Cache object, the OTel/Tehuti callbacks still fire on the
next 60-second collection cycle, dereferencing the stale JNI handle and
causing a SIGSEGV.

This caused 21+ JVM crashes across EI and prod over 6 days. 82% of
crashes were on the `collectionWindowExecutor-1` (OTel) or
`Async_Gauge_Executor` (Tehuti) metrics threads. The dominant crash
signature was a classic C++ vtable use-after-free: the freed Cache
memory was reused by the Java heap, so virtual method dispatch jumped
to garbage addresses containing Java class metadata.

The existing partition-level metrics (block-cache-capacity, etc.) are
safe because they go through `getRocksDBStatValue()` which is guarded
by `synchronized(hostedRocksDBPartitions)` and `readCloseRWLock`. The
RMD cache callbacks bypassed all of these guards.

Fix: Check `rmdCache.isOwningHandle()` before each JNI call. After
`RocksObject.close()`, `isOwningHandle()` returns false, preventing
the stale native pointer dereference.
Copilot AI review requested due to automatic review settings April 15, 2026 16:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes JVM crashes (SIGSEGV) caused by use-after-free when sampling RocksDB RMD block cache metrics via async gauge callbacks, by guarding JNI calls with an ownership check.

Changes:

  • Wrap RMD block cache usage and pinned_usage async gauge suppliers with an isOwningHandle() check and return 0 when the handle is non-owning.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Replace isOwningHandle() check with a stronger pattern: store the Cache
in a volatile field, and have callbacks read it into a local variable
before null-checking. The local pins the reference on the stack so it
cannot become null between check and JNI call — no locks needed.

Add closeRMDBlockCache() called from RocksDBStorageEngineFactory.close()
BEFORE sharedRMDCache.close() to ensure callbacks see null before native
memory is freed.
@sushantmane sushantmane changed the title Fix use-after-free in RMD block cache metrics callbacks [server][dvc] Fix use-after-free in RMD block cache metrics callbacks Apr 15, 2026
@sushantmane sushantmane added the needs-reviewer Looking for a reviewer to pick this up label Apr 15, 2026

@sixpluszero sixpluszero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the quick fix!

@sushantmane

Copy link
Copy Markdown
Contributor Author

Thanks for the review, @sixpluszero !

@sushantmane
sushantmane merged commit 49e1978 into linkedin:main Apr 15, 2026
106 checks passed
@sushantmane
sushantmane deleted the sumane/fix-rmd-cache-use-after-free branch April 15, 2026 18:40
@sushantmane sushantmane added approved PR has been approved and removed needs-reviewer Looking for a reviewer to pick this up labels Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved PR has been approved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants