Pewarta Proxy

📋 Server Request & Proxy Logs

Loading active proxy settings...
Developed by Khairul Adha (adhexa.id)
Mengambil log server...

⚙️ Pengaturan Proxy Target

Skip to content

SIGSEGV running tests #279

Open
Open

Description

@stroomdev66

Depending 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.

Activity

  1. added theissue type on Nov 12, 2025
  2. added
    bugAn issue relates to behaviour that violates the intention of the library
    on Nov 12, 2025
  3. changed the title [-]SegFault running tests[/-] [+]SIGSEGV running tests[/+] on Nov 12, 2025
  4. added a commit that references this issue on Nov 12, 2025
  5. stroomdev66 commented on Nov 12, 2025

    @stroomdev66
    CollaboratorAuthor

    See branch gh-279_segfault and TestSegfault to demo

  6. at055612 commented on Nov 12, 2025

    @at055612
    Collaborator

    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.

  7. bp-alex commented on Nov 12, 2025

    @bp-alex
    Member

    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.

  8. stroomdev66 commented on Nov 13, 2025

    @stroomdev66
    CollaboratorAuthor

    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.

  9. at055612 commented on Nov 13, 2025

    @at055612
    Collaborator

    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.

  10. bp-alex commented on Nov 13, 2025

    @bp-alex
    Member

    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.

  11. at055612 commented on Nov 14, 2025

    @at055612
    Collaborator

    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.

  12. wardle commented on Nov 14, 2025

    @wardle
    Contributor

    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.

  13. bp-alex commented on Nov 14, 2025

    @bp-alex
    Member

    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.

  14. at055612 commented on Nov 17, 2025

    @at055612
    Collaborator

    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
    
    
    Image

    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);
            }
        }
    }
  15. cdprete commented on Nov 18, 2025

    @cdprete
    Contributor

    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.

  16. 27 remaining items

  17. added 15 commits that reference this issue on Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugAn issue relates to behaviour that violates the intention of the library

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions