search

CoreyKaylor / Lightning.NET

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

Spanning new transactions and closing them is not thread safe #38

Closed KarolKolenda closed 9 years ago

KarolKolenda commented 9 years ago

It looks like BeginTransaction and Commit, Abort as well as OpenDatabase are not thread safe. They are probably safe on LMDB side but c# support classes like TransactionManager uses HashSet not wrapped with any lock. They fail when multiple readers are being created and destroyed on different threads in a short period of time.

buybackoff commented 9 years ago

There is possibly a deeper issue - internal locks are thread local, and the documentation mentions several times (e.g. in MDB_NOTLS)

A read-only transaction may span threads if the user synchronizes its use. Applications that multiplex many user threads over individual OS threads need this option.

Some synchronization is necessary on client side.

I am thinking about a custom scheduler and making all operations async. NetMQ has similar issues - underlying logic require sockets to work on the same thread. NetMQ uses a custom scheduler for that:

https://github.com/zeromq/netmq/blob/d439e00cb2c03450fc75b69f1a5c600e8ab7451e/src/NetMQ/NetMQScheduler.cs

KarolKolenda commented 9 years ago

Oh, I am thinking only about a single transaction per thread - I am not spanning it across multiple threads. Once I guarded BeginTransaction, Commit, Abort and OpenDatabase with locks everything is fine. The only errors I got before were from c# collections of various managers (TransactionManager for instance) - nothing really to do with LMDB binary.

Saying all that your solution would be brilliant and would allow passing transactions between threads. BTW: You did great job with this library - it is really amazing.

buybackoff commented 9 years ago

@KarolKolenda not me, the author is Ilya :)

KarolKolenda commented 9 years ago

:+1: Akward - me be stupid :)

buybackoff commented 9 years ago

@KarolKolenda I agree the library is amazing, I have learned a lot from this implementation of native interop and from its design!

ilyalukyanov commented 9 years ago

@KarolKolenda thank you! Defenitely this is a bug. Will fix soon.