-
Notifications
You must be signed in to change notification settings - Fork 24
feat(ffi): protect unsafe concurrent ops #1474
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
|
I don't know if Go has any read-write locks in stdlib, but this is a case where we need it. Most of the operations with the database handle are safe to execute concurrently (e.g., read operations). And, reading is also safe to do concurrently while committing. However, dropping is not safe to do while there are any concurrent reads or ongoing commit. So, before we drop, we need to acquire a semaphore that waits for ongoing reads to complete and prevents future reads, a lock to prevent commits, and a lock to prevent concurrent drops (which is handled by the keepAliveHandle and also why I wanted to name the function |
It does, my bad for not noticing this! I think I fixed this issue. |
| startKey, endKey Maybe[[]byte], | ||
| rootHash Hash, | ||
| maxLength uint32, | ||
| ) (Hash, error) { |
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 we also need to acquire the commit lock here. And should we rename it to commit lock? "State lock" is correct but wasn't my immediate thought when looking for it.
The goal of this is to prevent potentially unintended side effects of using the most recent revision in a concurrent manner. This isn't intended to remove the burden of correct concurrency from the user, but prevent easy mistakes. We expect these invariants to hold, so why not guarantee it?
Closes #1473