Skip to content
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

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

Open
muthukrishnan24 opened this issue Feb 4, 2020 · 5 comments
Open

Comments

@muthukrishnan24
Copy link

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

  • using endBytes var after iterate loop
  • using runtime.KeepAlive
  • turning off golang GC 😞

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
Copy link
Contributor

JelteF commented Feb 4, 2020

Seems I never made a PR back with this commit: GetStream@42987db

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

@flxflx
Copy link

flxflx commented Feb 7, 2020

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
Copy link

flxflx commented Feb 7, 2020

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

@JelteF
Copy link
Contributor

JelteF commented Feb 7, 2020

@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
Copy link

flxflx commented Feb 7, 2020

I agree!

flier added a commit to flier/gorocksdb that referenced this issue Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants