Repository navigation
Conversation
|
Hi @jamals86! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Summary:
TransactionDB.closeE(),OptimisticTransactionDB.closeE(), andTtlDB.closeE()do not release their owned column-family handles before closing and disposing the database. Theirclose()implementations, and both close variants in plainRocksDB, already do this cleanup. AlthoughcloseE()is documented as the exception-reporting equivalent ofclose(), calling it on these specialized databases leaves the returned Java handles owning native resources after database disposal. A subsequent handle close cannot perform the normal native cleanup becauseColumnFamilyHandle.disposeInternal()checks whether the database still owns its native handle.This patch performs the same owned-handle cleanup as
close(), before database ownership is released, in all three overrides. It adds a shared regression test covering plain RocksDB, TransactionDB, OptimisticTransactionDB, and TtlDB, with and without a manually closed column family, followed by repeated calls to both close variants.Motivation and scope:
Our Java application has experienced intermittent RocksDB JNI JVM crashes for a long time. Investigating those incidents led us to this separate, reproducible shutdown-cleanup inconsistency. We have not established this defect as the cause of those crashes, and this PR does not claim to fix the original production crash.
Callers must still stop concurrent operations and release other dependent resources before closing a database. This change does not add synchronization to read/write paths or make arbitrary concurrent use during shutdown safe.
Changes visible to existing users:
Owned column-family handles are now released by the three specialized
closeE()implementations, consistently withclose()and the base class. Public method signatures and native code are unchanged.Test Plan:
javac --release 8on Windows x64, Amazon Corretto 21.0.13.RocksDBTest.closeEClosesOwnedColumnFamilieswith assertions enabled against the rocksdbjni 11.8.1 Windows native library: passed all eight database/manual-close combinations. Repeated the test 100 times (800 scenario executions), with no failures.build_tools/check-sources.shandgit diff --check: passed.