Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java API Auto close iterators #13132

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

alanpaxton
Copy link
Contributor

@alanpaxton alanpaxton commented Nov 11, 2024

In an analogous way to owning column family handles, and closing them when the DB itself is closed, we make iterators be “child” objects of the DB.
Iterator lifecycle is generally shorter than DB, so we give them a lambda to remove themselves from the DB on iterator close().
Add tests to check that iterators which remain open when the DB is requested to close, are now closed when the DB is closed.

This fixes #5234

@alanpaxton alanpaxton marked this pull request as draft November 11, 2024 12:46
@alanpaxton alanpaxton force-pushed the eb/iterator-crash-on-closed-db branch from d8cdf34 to 0ae99e8 Compare November 11, 2024 15:22
@alanpaxton alanpaxton force-pushed the eb/iterator-crash-on-closed-db branch from 1b6f9ff to 4603e58 Compare November 19, 2024 16:42
Copy link

@ufochill ufochill left a comment

Choose a reason for hiding this comment

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

In an analogous way to owning column family handles, and closing them when the DB itself is closed, we make iterators be “child” objects of the DB.
Iterator lifecycle is generally shorter than DB, so we give them a lambda to remove themselves from the DB on iterator close().
Add a few tests to check that iterators which remain open when the DB is requested to close, are now closed when the DB is closed.

TODO - other DB subclasses need to add iterators to the list
TODO - there are cases where isOwningHandle() should not throw an exception; we need a less sledgehammer way of implementing this
TODO - fix the current Java tests
TODO - the atomic improvement in facebook#5234
@alanpaxton alanpaxton force-pushed the eb/iterator-crash-on-closed-db branch from 43b6e0c to 7aa0770 Compare December 2, 2024 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Iterator crashes JVM on closed database (RocksJava)
3 participants