CoreyKaylor / Lightning.NET

.NET library for LMDB key-value store
Other
398 stars 82 forks source link

Multithreading crash #72

Closed unrealwill closed 8 years ago

unrealwill commented 8 years ago

Hello, I'm having some crashes (double free in native library on transaction or database dispose) on a simple multi threading example. (It doesn't crash if I add a mutex on the dispose (but it's not cool because I can't use the using notation, and there is some mutex performance related issues)) Can you please advice ?

   public static void ReadTransact(LightningEnvironment env,int i)
    {
        using (var tx = env.BeginTransaction(TransactionBeginFlags.ReadOnly))
        {
            using( var db = tx.OpenDatabase("custom")) 
            {
            var result = tx.Get(db, Encoding.UTF8.GetBytes("hello"));
            //Console.WriteLine("{0} {1}", Thread.CurrentThread.ManagedThreadId,i);
            //Console.WriteLine(Encoding.UTF8.GetString(result));
            }

        }

    }

    public static void TestLMDBMultithreading()
    {
        //We create a test database 
        //Normally should be done in an other process to avoid opening an env twice in the same process but that's not the problem here
        var env = new LightningEnvironment("lmdbtest");
        env.MaxDatabases = 2;
        env.Open();

        using (var tx = env.BeginTransaction())
        using (var db = tx.OpenDatabase("custom", new DatabaseConfiguration { Flags = DatabaseOpenFlags.Create }))
        {
            tx.Put(db, Encoding.UTF8.GetBytes("hello"), Encoding.UTF8.GetBytes("world"));
            tx.Commit();
        }
        env.Flush(true);
        env.Dispose();

        //We create an environment for reading
        var readenv = new LightningEnvironment("lmdbtest");
        readenv.MaxDatabases = 2;
        readenv.Open(EnvironmentOpenFlags.ReadOnly);

        var parralelOpt = new ParallelOptions();
        //We use MaxDegreeOfParallelism to stay below env.MaxReaders
        parralelOpt.MaxDegreeOfParallelism = 1;

        var range = Enumerable.Range(0, 1000000);
        Parallel.ForEach(range, x=> ReadTransact(readenv,x) );

        Console.WriteLine("Done");
        readenv.Dispose();
        Console.WriteLine("Environment Disposed");
    }

It crashes with :

* Error in `/usr/bin/cli': double free or corruption (fasttop): 0x00007fee9c036ad0 * Stacktrace:

at <0xffffffff> at (wrapper managed-to-native) LightningDB.Native.LmdbMethods.mdb_dbi_close (intptr,uint) <0x00065> at LightningDB.Native.Lmdb.mdb_dbi_close (intptr,uint) <0x00017> at LightningDB.LightningDatabase.Dispose (bool) <0x000c7> at LightningDB.LightningDatabase.Dispose () <0x00015> at RdfStore.MainClass.ReadTransact (LightningDB.LightningEnvironment,int) <0x000d5> at RdfStore.MainClass/cAnonStorey0.<>m0 (int) <0x0001f> at System.Threading.Tasks.Parallel/cAnonStorey7`2<int, object>.<>m1 () <0x0049e> at System.Threading.Tasks.Task.InnerInvoke () <0x0004f> at System.Threading.Tasks.Task.InnerInvokeWithArg (System.Threading.Tasks.Task) <0x00013> at System.Threading.Tasks.Task/cAnonStorey0.<>m0 (object) <0x001cf> at System.Threading.Tasks.Task.InnerInvoke () <0x00088> at System.Threading.Tasks.Task.Execute () <0x00055> at System.Threading.Tasks.Task.ExecutionContextCallback (object) <0x00055> at System.Threading.ExecutionContext.RunInternal (System.Threading.ExecutionContext,System.Threading.ContextCallback,object,bool) <0x00178> at System.Threading.ExecutionContext.Run (System.Threading.ExecutionContext,System.Threading.ContextCallback,object,bool) <0x00020> at System.Threading.Tasks.Task.ExecuteWithThreadLocal (System.Threading.Tasks.Task&) <0x00116> at System.Threading.Tasks.Task.ExecuteEntry (bool) <0x000ee> at System.Threading.Tasks.Task.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem () <0x0000e> at System.Threading.ThreadPoolWorkQueue.Dispatch () <0x001d6> at System.Threading._ThreadPoolWaitCallback.PerformWaitCallback () <0x00008> at (wrapper runtime-invoke) .runtime_invoke_bool (object,intptr,intptr,intptr) <0x0005a>

Native stacktrace:

/usr/bin/cli() [0x4a77ca]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x10330) [0x7feec313f330]
/lib/x86_64-linux-gnu/libc.so.6(gsignal+0x37) [0x7feec2b89c37]
/lib/x86_64-linux-gnu/libc.so.6(abort+0x148) [0x7feec2b8d028]
/lib/x86_64-linux-gnu/libc.so.6(+0x732a4) [0x7feec2bc62a4]
/lib/x86_64-linux-gnu/libc.so.6(+0x7f55e) [0x7feec2bd255e]
[0x4143eaa6]

Debug info from gdb:

Could not attach to process. If your uid matches the uid of the target process, check the setting of /proc/sys/kernel/yama/ptrace_scope, or try again as the root user. For more details, see /etc/sysctl.d/10-ptrace.conf ptrace: Operation not permitted. No threads.

Got a SIGABRT while executing native code. This usually indicates a fatal error in the mono runtime or one of the native libraries

used by your application.

Aborted (core dumped)

CoreyKaylor commented 8 years ago

You need to also use the environment option for NoThreadLocalStorage when using the read-only transaction in order for multi-threading to be possible. You'll also want to reuse a new LightningDatabase instance from the 2nd environment you open rather than opening and disposing each time. Here's a link to the docs that describe in more detail (searching for "thread" helps as well). Tweaking those two things in the sample you provided I was able to run successfully with a MaxDegreeOfParallelism = 8.

http://lmdb.tech/doc/group__mdb.html#ga32a193c6bf4d7d5c5d579e71f22e9340

unrealwill commented 8 years ago

Thank you for your response. Unless I misunderstood you are suggesting using only 1 single read transaction doing its "get" in parallel in various threads. Your solution is working but has the drawback of being a long-lived read transaction.

My simplistic example and its solution was intended as a simple model for a web-service for which there may never be an end to the read stream, for which your solution would not read the newest values. By adding a mutex on Dispose

private static Mutex mut = new Mutex();

    public static void ReadTransact(LightningEnvironment env,int i)
    {

        var tx = env.BeginTransaction(TransactionBeginFlags.ReadOnly);

        var db = tx.OpenDatabase("custom");

        var result = tx.Get(db, Encoding.UTF8.GetBytes("hello"));
            //Console.WriteLine("{0} {1}", Thread.CurrentThread.ManagedThreadId,i);
            //Console.WriteLine(Encoding.UTF8.GetString(result));

        mut.WaitOne();
        db.Dispose();
        tx.Dispose();
        mut.ReleaseMutex();
    }

It works fine without the NoThreadLocalStorage option (The transaction is created inside a thread (though managed) and don't change thread so I don't think it's really necessary for my solution). Would it be possible to make the Dispose thread-safe (inside the library) ? Would it be possible to make it thread-safe using a lock-free non blocking (concurrent) collection or thread local storage in place of a mutex (and still provide some guarantee on the number of readers)?

Thank you

CoreyKaylor commented 8 years ago

I didn't say reuse the transaction. I said reuse the database. No need for long lived transactions. If you read the docs it says databases can be reused across transactions once the initial transaction that opened it is closed. That's what I'm referring to.

On Aug 6, 2016, at 6:48 PM, unrealwill notifications@github.com wrote:

Thank you for your response. Unless I misunderstood you are suggesting using only 1 single read transaction doing its "get" in parallel in various threads. Your solution is working but has the drawback of being a long-lived read transaction.

My simplistic example and its solution was intended as a simple model for a web-service for which there may never be an end to the read stream, for which your solution would not read the newest values. By adding a mutex on Dispose

private static Mutex mut = new Mutex();

public static void ReadTransact(LightningEnvironment env,int i)
{

    var tx = env.BeginTransaction(TransactionBeginFlags.ReadOnly);

    var db = tx.OpenDatabase("custom");

    var result = tx.Get(db, Encoding.UTF8.GetBytes("hello"));
        //Console.WriteLine("{0} {1}", Thread.CurrentThread.ManagedThreadId,i);
        //Console.WriteLine(Encoding.UTF8.GetString(result));

    mut.WaitOne();
    db.Dispose();
    tx.Dispose();
    mut.ReleaseMutex();
}

It works fine without the NoThreadLocalStorage option (The transaction is created inside a thread (though managed) and don't change thread so I don't think it's really necessary for my solution). Would it be possible to make the Dispose thread-safe (inside the library) ? Would it be possible to make it thread-safe using a lock-free non blocking (concurrent) collection or thread local storage in place of a mutex (and still provide some guarantee on the number of readers)?

Thank you

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.

unrealwill commented 8 years ago

Thank you so much, I hadn't understood that the database handle could be reused. By reading the doc of mdb_dbi_open more carefully, it says so and also explain that it should not be called concurrently (which means my solution was wrong). Your solution also has the added benefit of not spending time opening and closing the database which means it's faster.

So for future reference, and other people here is what works :

     public static void ReadTransact(LightningEnvironment env, LightningDatabase db,int i)
    {

        using( var tx = env.BeginTransaction(TransactionBeginFlags.ReadOnly) )
        {
            var result = tx.Get(db, Encoding.UTF8.GetBytes("hello"));
            //Console.WriteLine("{0} {1}", Thread.CurrentThread.ManagedThreadId,i);
            //Console.WriteLine(Encoding.UTF8.GetString(result));
            tx.Commit();
        }

    }

    public static void TestLMDBMultithreading()
    {
        //We create a test database 
        //Normally should be done in an other process to avoid opening an env twice in the same process but that's not the problem here
        var env = new LightningEnvironment("lmdbtest");
        env.MaxDatabases = 2;
        env.Open();

        using (var tx = env.BeginTransaction())
        using (var db = tx.OpenDatabase("custom", new DatabaseConfiguration { Flags = DatabaseOpenFlags.Create }))
        {
            tx.Put(db, Encoding.UTF8.GetBytes("hello"), Encoding.UTF8.GetBytes("world"));
            tx.Commit();
        }
        env.Flush(true);
        env.Dispose();

        //We create an environment for reading
        var readenv = new LightningEnvironment("lmdbtest");
        readenv.MaxDatabases = 2;
        readenv.MaxReaders = 10000;
        readenv.Open(EnvironmentOpenFlags.ReadOnly | EnvironmentOpenFlags.NoThreadLocalStorage);

        var parralelOpt = new ParallelOptions();
        //We use MaxDegreeOfParallelism to stay below env.MaxReaders
        parralelOpt.MaxDegreeOfParallelism = 1000;

        //We create a transaction to open the database
        LightningDatabase ldb; //In case of write transactions beware of not disposing it until all transactions referencing it are done see mdb_dbi_close
        using (var tx = readenv.BeginTransaction(TransactionBeginFlags.ReadOnly))
        {
            ldb = tx.OpenDatabase("custom");
            tx.Commit();
        }

        //We reuse the database handle
        var range = Enumerable.Range(0, 1000000);
        Parallel.ForEach(range, x=> ReadTransact(readenv,ldb,x) );

        Console.WriteLine("Done");
        readenv.Dispose();
        Console.WriteLine("Environment Disposed");
    }