GeezerVison is showing github.com — SIGSEGV running tests · Issue #279 · lmdbjava/lmdbjava
SIGSEGV running tests #279DescriptionDepending on how tests are run I am seeing an occasional segmentation fault. It happens in tests due to execution of subsequent tests, i.e. running tests on their own does not cause a segfault. I have tracked the issue down to tests that leave transactions unclosed, e.g. tests that create an env, open a transaction and then close the env without closing the transaction first.
The LmdbJava code deliberately prevents transactions from being used or closed after envs are closed as any use is expected to cause a segfault. The tests are designed to ensure that LmdbJava prevents this and tests the behaviour. However these tests leave LMDB in a state that will then cause a random segfault in some future scenario. I have a test case that demonstrates this happening quite reliably.
If I add code to force the env to track all open transactions and ensure they are closed before the env closes then there is no problem, however this would introduce some overhead keeping track of transactions in a way that is thread safe.
I think that the issue might be something to do with LMDB keeping track of transactions per thread as the issue is linked to the use of multiple threads. This is why it fails in some test execution scenarios that use multiple threads and not others. Note that it still fails with MDB_NOTLS used.
I'm not sure what the answer is. We obviously want to test that LmdbJava guards against incorrect usage but a situation that leads to intermittent segfault at some random later point is bad news. It might be something that can be fixed by the maintainer of LMDB if the problem is better understood at a deeper level. Activitychanged the title [-]SegFault running tests[/-] [+]SIGSEGV running tests[/+] on Nov 12, 2025stroomdev66 commented on Nov 12, 2025 CollaboratorAuthor More actions See branch gh-279_segfault and TestSegfault to demo at055612 commented on Nov 12, 2025 From the lmdb docs for mdb_env_close (http://www.lmdb.tech/doc/group__mdb.html#ga4366c43ada8874588b6a62fbda2d1e95):
Close the environment and release the memory map.
Only a single thread may call this function. All transactions, databases, and cursors must already be closed before calling this function. Attempts to use any such handles after calling this function will cause a SIGSEGV. The environment handle will be freed and must not be used again after this call.
While it would be nice for LmdbJava to track all the open txns and prevent the Env from closing while any are open, I think this would be just too much overhead on the opening/closing of txns which is a very hot path.
I think the javadoc on the Env#close method should be improved to make it very clear that closing the env when any dbs/txns/cursors are open may result in a SIGSEGV. The responsibility is then on the caller to adhere to that contract.
Unless @benalexau has any other ideas, I also think the problem tests that try to assert that txns can't be closed after the env is closed ought to be removed if we have no way to clean up the invalid state that they have created. They are trying to test something that the contract is stating not to do. bp-alex commented on Nov 12, 2025 I agree we should improve the documentation.
We can use an AtomicInteger reference counting pattern:
- Increment on
Txn constructor
- Decrement on
Txn.close()
- Block on
Env.close() if count is non-zero
Wrap the reference counting behind our existing Env.SHOULD_CHECK mechanism so users can set lmdbjava.disable.checks to opt out. Since it's a static, the JVM optimizes it away anyway.
We should benchmark to confirm, but I'd be surprised if AtomicInteger operations (especially uncontended) measurably impact latency compared to FFI and I/O overhead for the transaction itself. stroomdev66 commented on Nov 13, 2025 CollaboratorAuthor More actions I'll create a PR that uses AtomicInteger to count open Txns that will throw an exception if count > 0 on env close. I will probably have to alter the tests so we aren't breaking the contract deliberately in tests as it is often causing other tests to fail. I'll also add the same open txn count protection to parent Txns to ensure all child Txns are closed before the parent as this also causes the same sort of problems. I'll make sure all of the new behaviour is within Env.SHOULD_CHECK.
I'll wait until AT has his big PR merged as I don't want to add complexity to that change. at055612 commented on Nov 13, 2025 I wonder if we can use LongAdder instead of AtomicInteger. LongAdder is better in high concurrency situations as it holds the total in multiple 'buckets' to reduce contention. We only care about the total sum() in Env.close.
public long sum()
Returns the current sum. The returned value is NOT an atomic snapshot; invocation in the absence of concurrent updates returns an accurate result, but concurrent updates that occur while the sum is being calculated might not be incorporated.
However, the private boolean closed; field and checkNotClosed() should ensure that no Txn is opened once close() has been called. I am pretty sure closed ought to be volatile or an AtomicBoolean though. Reacted by Cosimo Damiano Prete bp-alex commented on Nov 13, 2025 Agreed, closed should be volatile. Re LongAdder, I don't think we are likely to have high contention situations. The extra complexity of accumulating the cells seems likely to exceed the simple CAS operation underpinning AtomicInteger. Plus we have SHOULD_CHECK, so if performance really became a concern the user can switch the entire mechanism off. at055612 commented on Nov 14, 2025
I don't think we are likely to have high contention situations.
I'm not sure that is true. One of our uses of LJ is for lookups of reference data to decorate event streams. Many threads (some servers have 100+ cores) will each be processing event streams, and each process will be doing many 000s of lookups (open read txn, dbi.get, close txn) on the same env. There will almost certainly be contention on the txn reference counter.
These benchmarks would suggest that you only need 2 threads in play for LongAdder to show a significant benefit and beyond 2 it is a clear winner. It does however confirm that for a single thread there is a cost. Ultimately, we need to benchmark many threads all doing open-get-close for LongAdder vs AtomicInteger to see how they perform in a more representative scenario.
http://blog.palominolabs.com/2014/02/10/java-8-performance-improvements-longadder-vs-atomiclong/
One option would be for the Env builder to have something like
Env.create()
...
.singelThreaded()
.open(...) so that the Env knows if it needs to be thread safe or not. Some uses of LJ may be purely single threaded and thus don't need/want the added overhead of concurrency protection. The Env can then be constructed to suit that setting, e.g. an int based ref counter vs a LongAdder based one.
People will use LJ in all sorts of different ways so if the Env/Dbi has more information about how they are being used (e.g. single/multi threaded, fixed/variable length keys), they can then include minor optimisations to cater to the use case. wardle commented on Nov 14, 2025 Can I play Devil's advocate here?
In my usage of lmdbjava, I open an environment at the beginning, my system runs, and then the environment is closed as the application is shutdown. Given the documentation, if I were opening and closing the environment repeatedly, I would expect to manage the transactions and ensure all are closed before trying to close the environment. I'm usually not keen on fixing tests to make them work, but in this case, isn't it simply the case that the tests are doing the wrong thing? We're running lmdbjava for its excellent performance, and, as has been discussed previously, some of that performance comes with 'foot guns' in terms of currently unsafe methods and an ability to create segfaults not possible without the use of native code.
It sounds as if the suggested approach of counting transactions will stop the errors during test running, but I am unaware of any reports of this kind of issue from users of lmdbjava in live usage, probably because those usages close all transactions. Most of my transactions are extremely short, and any server code would not shutdown until all threads, and therefore all transactions have ended.
I think in most real-world usage, surely closing the environment would occur only a) at application shutdown, or b) as part of a lifecycle in which all threads using that environment will have ended because one has built a proper shutdown process?
While the counting of transactions will likely not add significant overhead, it does add more complexity, and I think I'd be in favour of simply fixing the tests to use more appropriate behaviour [and therefore be better examples for others to follow] and improve the documentation. bp-alex commented on Nov 14, 2025
I'm not sure that is true. One of our uses of LJ is for lookups of reference data to decorate event streams. Many threads (some servers have 100+ cores) will each be processing event streams, and each process will be doing many 000s of lookups (open read txn, dbi.get, close txn) on the same env. There will almost certainly be contention on the txn reference counter.
Why not use a long-running Txn for each thread? It doesn't sound like there are many mutations (being reference data) and that's when you'd generally want to be more careful with long-running transactions. A heavy, read-dominated workload would usually benefit from a long-running Txn due to fewer allocations, GC pressure, and FFI crosses associated with managing the Txn handle.
Still, I acknowledge your broader point that sometimes there may be lots of threads running on many cores.
These benchmarks would suggest that you only need 2 threads in play for LongAdder to show a significant benefit
I note that benchmark was run under Java 8 circa 2014. We should probably re-run it on modern JVMs and compare AtomicInteger, AtomicLong, and LongAdder. Would anyone be able to run this?
.... an ability to create segfaults not possible without the use of native code
Like most C libraries, LMDB documents many requirements in order to avoid SIGSEGV. Quoting our package-info.java:
The classes in LmdbJava DO NOT provide any concurrency guarantees. Instead you MUST observe
LMDB's specific thread rules (eg do not share transactions between threads). LmdbJava does not
shield you from these requirements, as doing so would impose locking overhead on use cases that
may not require it or have already carefully implemented application threading (as most low
latency applications do to optimize the memory hierarchy, core pinning etc).
That said, if we can add safety checks that are zero-cost when SHOULD_CHECK is disabled, and the code delta is small, it seems worth doing to help users avoid fatal errors where we can.
I think I'd be in favour of simply fixing the tests to use more appropriate behaviour [and therefore be better examples for others to follow] and improve the documentation.
Agreed. Let's fix any tests that don't close transactions properly or document why they deliberately don't. at055612 commented on Nov 17, 2025
We should probably re-run it on modern JVMs and compare AtomicInteger, AtomicLong, and LongAdder.
That benchmark repo was so out of date I ended up just crafting a quick jmh class to test AI, AL, LA and a primitive int (for reference and accepting that it is pointless where threads >1). This is using Java 21 temurin on a 12core 24thread cpu.
=== ONE THREAD ===
Benchmark Mode Cnt Score Error Units
TestAtomicInteger.atomicInteger thrpt 633670967.338 ops/s
TestAtomicInteger.atomicLong thrpt 632073002.140 ops/s
TestAtomicInteger.longAdder thrpt 525088771.702 ops/s
TestAtomicInteger.primitiveInt thrpt 2805891997.822 ops/s
=== TWO THREADS ===
Benchmark Mode Cnt Score Error Units
TestAtomicInteger.atomicInteger thrpt 124344537.837 ops/s
TestAtomicInteger.atomicLong thrpt 92389992.099 ops/s
TestAtomicInteger.longAdder thrpt 933091591.699 ops/s
TestAtomicInteger.primitiveInt thrpt 241382219.430 ops/s
=== TEN THREADS ===
Benchmark Mode Cnt Score Error Units
TestAtomicInteger.atomicInteger thrpt 89816052.534 ops/s
TestAtomicInteger.atomicLong thrpt 83321174.667 ops/s
TestAtomicInteger.longAdder thrpt 4233313651.993 ops/s
TestAtomicInteger.primitiveInt thrpt 188787439.079 ops/s
This confirms that for all but single threaded operation, LA is orders of magnitude faster than AI/AL even with only 2 threads in play.
If the Env.Builder were to have a .singleThreaded() method then the txn ref counter could use int for singlethreaded mode and LA for everything else.
@Threads(2)
public class TestAtomics {
public static final int FORKS = 1;
public static final int WARMUPS = 1;
public static final int ITERATIONS = 1;
@Fork(value = FORKS, warmups = WARMUPS)
@Benchmark
@BenchmarkMode(Mode.Throughput)
@Measurement(iterations = ITERATIONS)
public void atomicLong(final MyAtomicLong state, final Blackhole blackhole) {
blackhole.consume(state.atomicLong.incrementAndGet());
}
@Fork(value = FORKS, warmups = WARMUPS)
@Benchmark
@BenchmarkMode(Mode.Throughput)
@Measurement(iterations = ITERATIONS)
public void atomicInteger(final MyAtomicInteger state, final Blackhole blackhole) {
blackhole.consume(state.atomicInteger.incrementAndGet());
}
@Fork(value = FORKS, warmups = WARMUPS)
@Benchmark
@BenchmarkMode(Mode.Throughput)
@Measurement(iterations = ITERATIONS)
public void longAdder(final MyLongAdder state, final Blackhole blackhole) {
state.longAdder.increment();
}
@Fork(value = FORKS, warmups = WARMUPS)
@Benchmark
@BenchmarkMode(Mode.Throughput)
@Measurement(iterations = ITERATIONS)
public void primitiveInt(final MyInt state, final Blackhole blackhole) {
state.anInt++;
}
@State(Scope.Benchmark)
public static class MyAtomicLong {
final AtomicLong atomicLong = new AtomicLong();
@TearDown(Level.Trial)
public void doTearDown() {
System.out.println(atomicLong.get());
}
}
@State(Scope.Benchmark)
public static class MyAtomicInteger {
final AtomicInteger atomicInteger = new AtomicInteger();
@TearDown(Level.Trial)
public void doTearDown() {
System.out.println(atomicInteger.get());
}
}
@State(Scope.Benchmark)
public static class MyLongAdder {
final LongAdder longAdder = new LongAdder();
@TearDown(Level.Trial)
public void doTearDown() {
System.out.println(longAdder.sum());
}
}
@State(Scope.Benchmark)
public static class MyInt {
int anInt = 0;
@TearDown(Level.Trial)
public void doTearDown() {
System.out.println(anInt);
}
}
}cdprete commented on Nov 18, 2025
Can I play Devil's advocate here?
In my usage of lmdbjava, I open an environment at the beginning, my system runs, and then the environment is closed as the application is shutdown. Given the documentation, if I were opening and closing the environment repeatedly, I would expect to manage the transactions and ensure all are closed before trying to close the environment. I'm usually not keen on fixing tests to make them work, but in this case, isn't it simply the case that the tests are doing the wrong thing? We're running lmdbjava for its excellent performance, and, as has been discussed previously, some of that performance comes with 'foot guns' in terms of currently unsafe methods and an ability to create segfaults not possible without the use of native code.
It sounds as if the suggested approach of counting transactions will stop the errors during test running, but I am unaware of any reports of this kind of issue from users of lmdbjava in live usage, probably because those usages close all transactions. Most of my transactions are extremely short, and any server code would not shutdown until all threads, and therefore all transactions have ended.
I think in most real-world usage, surely closing the environment would occur only a) at application shutdown, or b) as part of a lifecycle in which all threads using that environment will have ended because one has built a proper shutdown process?
While the counting of transactions will likely not add significant overhead, it does add more complexity, and I think I'd be in favour of simply fixing the tests to use more appropriate behaviour [and therefore be better examples for others to follow] and improve the documentation.
I actually have a very similar setup:
- Open the Envs (I've 32) on startup
- Open and use a tx-write every 20s for each Env
- Open and use a tx-read every 40s for each Env
@benalexau long-living tx-reads are only good if the writes are really rare.
Moreover, at least to my experience with this so far, as soon as a tx-write is open and a tx-read starts then the space in the Env file starts to grow quite a lot due to the copy-on-write mechanism LMDB has, ending up potentially in some LMDB error like MapSizeExeeded or so.
For example, I'm not able to provide a potential API exposing the data I've in LMDB exactly for this behavior.
It sounds as if the suggested approach of counting transactions will stop the errors during test running, but I am unaware of any reports of this kind of issue from users of lmdbjava in live usage, probably because those usages close all transactions.
@wardle I kind of had similar issues like the one presented in here in the past and I still have randomly from time to time, but it's difficult for me to say it's because of some race condition on closing the resources or because of something else. added 15 commits that reference this issue on Aug 10, 2026gh-279 Fix catch in Cursor ctor gh-279 Tidy asserts in TxnTest gh-279 Add Env.tryClose, add more tests gh-279 Add test for Cursor ctor exception gh-279 Remove dead code, add test for code coverage gh-279 Add RefCounter.run test gh-279 Fix AbstractFlagSetTest gh-279 Changes to AbstractFlagSetTest for codecov gh-279 Add tests to TargetNameTest gh-279 Change safeClose to only track RW cursors gh-279 Add more tests, improve javadoc
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|
