Skip to content

Gh 279 env close protection - #293

Open
at055612 wants to merge 62 commits into
masterfrom
gh-279-env-close-protection
Open

Gh 279 env close protection#293
at055612 wants to merge 62 commits into
masterfrom
gh-279-env-close-protection

Conversation

@at055612

@at055612 at055612 commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Addresses #279

The aim of this PR is to add optional protection to prevent the Env from being closed if there are transactions or cursors open. #279 discussed some options for implementing this protection using reference counting including using a simple AtomicInteger and LongAdder. LongAdder is no-go as it is not thread safe in the way we need to use it.

This PR uses a reference counter that uses an array of AtomicIntegers for improved multi threaded performance. This is similar to what LongAdder does, but adds in concurrency controls to allow a count to happen in a thread safe way. I have various tests to check this and AI has checked over it, but would welcome anyone picking holes in it in case I have missed some race condition.

Rather than conflate this protection with the existing SHOULD_CHECK logic, it is enabled via these four new Env.Builder methods:

Env.create()
  .setSafeClose() // default is NO close protection if not called
  .setSafeClose(true) // default is NO close protection if not called
  .setSingleThreaded()  // default is multithreaded if not called
  .setSingleThreaded(true) // default is multithreaded if not called

It may be beneficial to add a new system property to turn on/off safeClose to allow it to be turned on in dev, however users of the library can create their own system property to control the value for setSafeClose(enabled).

If safeClose and singleThreaded are set, a non-thread-safe reference counter is used that relies on a simple int and no concurrency protection. This will be faster and use less memory.

IfsafeClose and singleThreaded are both un-set, the Env will use a no-op implementation of RefCounter. I would hope that the JVM will optimise these no-op method calls out to remove the tiny overhead that they add.

If only safeClose is set, the striped thread safe reference counter implementation will be used.

If only singleThreaded is set, this has no impact on the Env, as this flag currently only impacts the choice of reference counter impl. It is possible that the singleThreaded flag on the env could also be used in the future to control how LMDBJava implements other things when used in a strictly single threaded way.

When safeClose is set, calling Env.close() will result in a EnvInUseException being thrown if there are open cursors/txns. It will not block or wait.

I started this PR quite a while ago, but have changed it to adopt some of the builder changes from #289. I disagree with the close(timeout) in #289 on the basis that it should be the responsibility of the calling code to determine when to close the env, rather than LMDBJava's.

I have added a tryClose() method which returns true if the close could be performed or false otherwise. With safeClose enabled, this means tryClose() will return false if the env is in use rather than throwing.

Comment thread src/test/java/org/lmdbjava/EnvTest.java Fixed
Comment thread src/test/java/org/lmdbjava/EnvTest.java Fixed
Comment thread src/test/java/org/lmdbjava/TestUtils.java Fixed
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.

2 participants