-
Notifications
You must be signed in to change notification settings - Fork 652
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
page already freed #72
Comments
/cc @SaranBalaji90 would you like to investigate into this one? this might be a good start for you to understand the storage layer of etcd. |
Hello! Just a random passerby, but I wanted to point something out (disclaimer, I haven't run this code). It looks like there is a race condition in the code that could be causing this. The byte slice being sent/received from the channel, appears to be racing between rand.Read and b.Put. While the channel send makes a copy of the slice header, the underlying array is not copied, so rand.Read is writing into the slice while b.Put is reading from it. |
Even if there is a race in application code, bolt should not panic right? Blotdb panics with an internal inconsistent state. |
@iaburton
When internal state is inconsistent, panic is best thing to do. There is modified example without race, that works fine: package main
import (
"log"
"math/rand"
"github.com/coreos/bbolt"
// "github.com/boltdb/bolt"
)
var BC = make(chan []byte)
func main() {
go Add(BC, "/opt/test.db")
s1 := make([]byte, 30)
s2 := make([]byte, 30)
for i := 0; i < 100000000; i++ {
rand.Read(s1)
BC <- s1
s1,s2 = s2,s1
}
close(BC)
}
func Add(c chan []byte, bfile string) {
const bname = `bc41`
db, err := bolt.Open(bfile, 0600, nil)
if err != nil {
log.Fatal(err)
}
defer db.Close()
err = db.Update(func(tx *bolt.Tx) error {
_, err := tx.CreateBucketIfNotExists([]byte(bname))
if err != nil {
return err //fmt.Errorf("create bucket: %s", err)
}
return nil
})
if err != nil {
log.Fatal(err)
panic(err)
}
more := true
for more {
err := db.Update(func(tx *bolt.Tx) error {
b := tx.Bucket([]byte(bname))
for i := 0; i < 100000; i++ {
v := <-c
if v == nil {
more = false
return nil
}
err = b.Put(v, []byte("0"))
if err != nil {
return err
}
}
return nil
})
if err != nil {
log.Fatal(err)
panic(err)
}
}
} Please close this issue. |
Ideally races happen in application code will not panic bolt, and lead it to a corrupted and unrecoverable state. This issue is a common user buffer problem. The proper way to handle it is to copy, check before use any user buffer. At least, current behavior should be documented. @djadala |
I'm not sure what else bolt should do here in this kind of race, but I think the current behavior is fine, it certainly makes the problem more visible. The implementation of b.Put does copy the key before storing it, which isn't mentioned in the documentation, perhaps that's worthy of including. The docs do say that the value must be valid the entire time, so it sort of implies that the key doesn't have to. |
The problem is that no one will enable race detector for production usage. I am fine with boltdb panics itself when there is a race. I am not so happy about the state being corrupted with |
I re-read the doc. It does mention this accurately. Thanks for pointing this out. |
True they won't for production, but they should during testing. And that is unfortunate I agree. There is another issue on the original bolt about an integrity checking function, perhaps that might be a solution. I cannot think of a way to keep this from happening (at any point during runtime) while also not penalizing correctly behaving applications. I don't think most packages (depending on what they do) should concern themselves with the application racing, so long as they don't race (the packages), and their API is reasonable. If the application races and not the package, that's up to the programmer of the "main" application to detect/fix/handle (this includes if the application race adversely effects the packages it imports, as it did here). Granted, I haven't used bolt since the fork so I'm just brainstorming, something may have changed I'm not aware of 😄 |
panic message can be changed from: |
As this is not bug in bolt, but race condition in programs that use bolt, i close this. |
Openig same issue as boltdb/bolt#731, because coreos/bbolt also crashes in same way:
The text was updated successfully, but these errors were encountered: