tecbot / gorocksdb

gorocksdb is a Go wrapper for RocksDB
http://rocksdb.org
MIT License
937 stars 269 forks source link

Unexpected Behaviour: golang GC clears variable passed to rocksdb #190

Open muthukrishnan24 opened 4 years ago

muthukrishnan24 commented 4 years ago

small reproducible go program here (not runnable in golang play) https://play.golang.org/p/B_1g13pr33d

in line 64, endBytes (byte array) is set to IterateUpperBound in gorocksdb, byteToChar util (using unsafe pointer) is used to pass the endBytes to rocksdb C Api

before the iterate loop (in line 74), completes, endBytes variable is GCed. which makes the comparator, to receive random bytes and the iterator randomly ends before reaching the actual upper_bound value.

similar unexpected behaviour occurs in slice.Data(), when store a reference of that byte and using it elsewhere

small hacks which worked (prevent GC of endBytes until loop ends) are

possible solution to make a copy of the endBytes in ReadOptions struct and pass it to rocksdb C Api or any alternate solution exists?

@tecbot @jamesbibby

JelteF commented 4 years ago

Seems I never made a PR back with this commit: https://github.com/GetStream/gorocksdb/commit/42987db69212a5231fdbf954dfe035d982cee2d7

Feel free to make a PR with that change, I don't have time for that anymore these days.

flxflx commented 4 years ago

FWIW, I ran into same issue. The general problem seems to affect large parts of the API. For example, batched writing. (K/V buffers may be GC'ed before the batch is written.)

EDIT: actually, WriteBatch doesn't seem to affected, because rocksdb internally copies the puts.

flxflx commented 4 years ago

I believe there should be a warning in the README.md

JelteF commented 4 years ago

@flxflx any other places where this happens is most certainly a bug IMHO, since keeping a pointer to GC owned memory is not allowed by CGO rules. Most inputs are copied by rocksdb internally. AFAIK set_iterate_upper_bound is one of the exceptions.

flxflx commented 4 years ago

I agree!