-
Notifications
You must be signed in to change notification settings - Fork 63
Implement Send on Cursors so we can move them between threads #334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
can we get this merged soon as I really want to use this! |
Hey @xav-db 👋 I cannot merge this as-is, as the behavior could break the wrapper and some invariants. However, I suggest you test that independently and tell me how your program is doing. Ultimately, I'd like a review from someone more knowledgeable than me on C stuff and thread safety. Take care 🌵 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think implementing Send
should be fine, the docs start with saying "The library is fully thread-aware".
Also, if desired, Sync
can be implemented on any type without methods that take &self
, for free. See std::sync::Exclusive for more info.
Hey @antonilol 👋
Unfortunately, you cannot trust the sole documentation of a C library. I don't know what "The library is fully thread-aware" exactly means but here is an example of non-thread awareness. Another is that it is not valid to implement
Very interesting, thanks. However, this is still nightly. We cannot rely on this in heed. At least, for now. |
Isn't this impossible already because to use a cursor,
I meant this as an example for how |
This is true, we will have to wait until the author of LMDB responds or someone checks the C code cursors use. |
going to go through the C code |
This PR implements
Send
on the LMDB cursors struct wrapper. The goal is to move database iterators between threads.If this PR is merged, we must release a v0.22.1 patch. This change is not breaking.
Hey @hyc 👋
Is it valid to move cursors between threads? Nothing in the documentation seems to prevent that, and nothing in the code prevents it either. Note that cursors are always deallocated when dropped (going out of scope). So, I only fear that we create a cursor on one thread and deallocate it on another. However, allocations are done with malloc, so it looks safe.
Thank you and have a lovely week 🍀