-
Notifications
You must be signed in to change notification settings - Fork 646
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
bucket.Put: copy key before seek #637
Conversation
6c28521
to
1d491a7
Compare
Signed-off-by: Wei Fu <[email protected]>
1d491a7
to
215e351
Compare
tests/failpoint/db_failpoint_test.go
Outdated
// When application inserts key/value, that key can be changed by application | ||
// during insertion. bbolt should copy key before seeking and updating. Otherwise, | ||
// some branch node could point to freed page. |
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.
It doesn't necessarily cause elements to point freed pages. In this test case, if you change the key to a larger key e.g. 200, the case will pass.
// When application inserts key/value, that key can be changed by application | |
// during insertion. bbolt should copy key before seeking and updating. Otherwise, | |
// some branch node could point to freed page. | |
// When bbolt is processing a `Put` invocation, the key might be concurrently | |
// updated by the application which calls the `Put` API (although it shouldn't). | |
// It might lead to a situation that bbolt use an old key to find a proper | |
// position to insert the key/value pair, but actually inserts a new key. | |
// Eventually it might break the rule that all keys should be sorted. In a | |
// worse case, it might cause page elements to point to already freed pages. |
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.
Thanks for the comment. Updated
Signed-off-by: Wei Fu <[email protected]>
Application might change key value after seeking and before real put. This unexpected behaviour could corrupt database. When users file issue, maintainers doesn't know application behaviour. It could be caused by data race. This patch is to prevent such case and save maintainers' time. Signed-off-by: Wei Fu <[email protected]>
215e351
to
a05ec68
Compare
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.
It's follow-up of etcd-io#637. Signed-off-by: Wei Fu <[email protected]>
It's follow-up of etcd-io#637. Signed-off-by: Wei Fu <[email protected]>
It's follow-up of etcd-io#637. Signed-off-by: Wei Fu <[email protected]> (cherry picked from commit 62d8026) Signed-off-by: Wei Fu <[email protected]>
- [bucket: allow to allocate key on stack in Put()](etcd-io#550) - [bucket.Put: copy key before seek](etcd-io#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io#641) Signed-off-by: Wei Fu <[email protected]>
- [bucket: allow to allocate key on stack in Put()](etcd-io#550) - [bucket.Put: copy key before seek](etcd-io#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io#641) Signed-off-by: Wei Fu <[email protected]>
It's follow-up of etcd-io#637. Signed-off-by: Wei Fu <[email protected]>
It's follow-up of etcd-io#637. Signed-off-by: Wei Fu <[email protected]>
- [bucket: allow to allocate key on stack in Put()](etcd-io#550) - [bucket.Put: copy key before seek](etcd-io#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io#641) Signed-off-by: Wei Fu <[email protected]>
- [bucket: allow to allocate key on stack in Put()](etcd-io/bbolt#550) - [bucket.Put: copy key before seek](etcd-io/bbolt#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io/bbolt#641) Signed-off-by: Wei Fu <[email protected]>
It's follow-up of etcd-io#637. Signed-off-by: Wei Fu <[email protected]> Signed-off-by: samuelbartels20 <[email protected]>
It's follow-up of etcd-io#637. Signed-off-by: Wei Fu <[email protected]> Signed-off-by: samuelbartels20 <[email protected]>
- [bucket: allow to allocate key on stack in Put()](etcd-io#550) - [bucket.Put: copy key before seek](etcd-io#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io#641) Signed-off-by: Wei Fu <[email protected]> Signed-off-by: samuelbartels20 <[email protected]>
Application might change key value after seeking and before real put.
This unexpected behaviour could corrupt database. When users file issue,
maintainers doesn't know application behaviour. It could be caused by
data race. This patch is to prevent such case and save maintainers' time.
REF: #72