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

Exception with cursor.Get(k, nil, Set) #96

Closed
slav opened this issue Jan 16, 2017 · 5 comments
Closed

Exception with cursor.Get(k, nil, Set) #96

slav opened this issue Jan 16, 2017 · 5 comments
Labels
Milestone

Comments

@slav
Copy link

slav commented Jan 16, 2017

I have db with dup values (not sure if relevant). When I'm trying to do k, v, e = cursor.Get(buf.Bytes(), nil, lmdb.Set) I get exception (see below). With k, v, e = cursor.Get(buf.Bytes(), nil, lmdb.SetKey) it works. Just Set positions cursor at the key but doesn't read value. But cursor.Get still attempts to read it which leads to the exception.

panic: runtime error: cgo argument has Go pointer to Go pointer

goroutine 21 [running]:
panic(0x56f360, 0xc0420b0260)
        C:/Go/src/runtime/panic.go:500 +0x1af
github.com/bmatsuo/lmdb-go/lmdb._cgoCheckPointer0(0xc0420bc024, 0x0, 0x0, 0x0, 0x0)
        ??:0 +0x60
github.com/bmatsuo/lmdb-go/lmdb.getBytesCopy(0xc0420b0170, 0xc0420b0170, 0x23190e0, 0x556ee0)
        D:/Projects/Go/src/github.com/bmatsuo/lmdb-go/lmdb/val.go:120 +0x51
github.com/bmatsuo/lmdb-go/lmdb.(*Txn).bytes(0xc0420b6120, 0xc0420b0170, 0x25, 0x40, 0xf)
        D:/Projects/Go/src/github.com/bmatsuo/lmdb-go/lmdb/txn.go:276 +0x7a
github.com/bmatsuo/lmdb-go/lmdb.(*Cursor).Get(0xc0420b01a0, 0xc0420bc024, 0x25, 0x40, 0x0, 0x0, 0x0, 0xf, 0xa0, 0x61, ...)
        D:/Projects/Go/src/github.com/bmatsuo/lmdb-go/lmdb/cursor.go:155 +0x147
github.com/bmatsuo/lmdb-go/lmdbscan.(*Scanner).Set(0xc0420ba120, 0xc0420bc024, 0x25, 0x40, 0x0, 0x0, 0x0, 0xf, 0x4c)
        D:/Projects/Go/src/github.com/bmatsuo/lmdb-go/lmdbscan/scanner.go:74 +0xc4
github.com/bmatsuo/lmdb-go/lmdbscan.(*Scanner).SetNext(0xc0420ba120, 0xc0420bc024, 0x25, 0x40, 0x0, 0x0, 0x0, 0xf, 0x9, 0x25)
        D:/Projects/Go/src/github.com/bmatsuo/lmdb-go/lmdbscan/scanner.go:85 +0x9b
bitbucket.org/agileharbor/lmdbbenchmark/lmdbLayer.(*Reader).ReadDupRecords.func1(0xc0420b6120, 0x0, 0x0)
        D:/Projects/Go/src/bitbucket.org/agileharbor/lmdbbenchmark/lmdbLayer/reader.go:103 +0x83d
github.com/bmatsuo/lmdb-go/lmdb.(*Env).run(0xc042072040, 0x412c00, 0x20000, 0xc0420ae0f0, 0x0, 0x0)
        D:/Projects/Go/src/github.com/bmatsuo/lmdb-go/lmdb/env.go:461 +0x157
github.com/bmatsuo/lmdb-go/lmdb.(*Env).View(0xc042072040, 0xc0420ae0f0, 0x9, 0xc000000002)
        D:/Projects/Go/src/github.com/bmatsuo/lmdb-go/lmdb/env.go:423 +0x4a
@bmatsuo
Copy link
Owner

bmatsuo commented Jan 22, 2017

That is a strange error. The function causing that panic is C.GoBytes(), which seems very strange.

The error is probably because lmdb.Set will not actually read values from the database, as you said. So whatever value lmdb-go is trying to extract wouldn't be valid anyway. I will add a test case for this to reproduce.

I will probably end up having the Cursor.Get method check the cursor op so it can short circuit and return nil slices when op is lmdb.Set.

AFAIK the op in question is the only one which does not return data. Are you aware of any other ops like this @slav?

http://www.lmdb.tech/doc/group__mdb.html#ga1206b2af8b95e7f6b0ef6b28708c9127

Thanks for the report.

@bmatsuo
Copy link
Owner

bmatsuo commented Jan 22, 2017

Interestingly, on my primary Linux machine lmdb.Set and lmdb.SetKey seem to operate the same (they return the same values).

I notice that you are running your program in a Windows environment. Perhaps you could run the tests for me on your computer and confirm that the test case I added does in fact reproduce the panic for you? It has been some time since I compiled lmdb-go on Windows myself, so I will have to try to get that environment running again.

Branch: bmatsuo/cursor-op-set-testing (commit c9757f4)

@slav
Copy link
Author

slav commented Jan 27, 2017

All the tests in branch "bmatsuo/cursor-op-set-testing" pass on windows.

I'm also unaware which ops init key/data and which don't. Documentation is only specific about MDB_SET_KEY to return key + data.

@bmatsuo
Copy link
Owner

bmatsuo commented Jan 28, 2017

OK. Thanks for checking that, @slav. I understand what is happening now. This is an issue with the bytes.Buffer type that I have run into before in #56. I can reproduce the error now and I will work on a fix.

bmatsuo added a commit that referenced this issue Jan 28, 2017
@bmatsuo
Copy link
Owner

bmatsuo commented Jan 28, 2017

I have pushed a commit to that branch (bmatsuo/cursor-op-set-testing). I have a working fix that I will upload to another branch and issue a pull request to respect the process. Thanks again for your help, @slav. It is very much appreciated.

@bmatsuo bmatsuo added the bug label Jan 28, 2017
@bmatsuo bmatsuo added this to the v1.8.0 milestone Jan 28, 2017
bmatsuo added a commit that referenced this issue Jan 28, 2017
Fixes #96

Issue #56 illustrated that working with Go memory using "C-style" memory
does not work well in modern Go runtimes.  That problem was thought to
be properly addressed, but #96 showed a lingering case where Set does
not modify MDB_val key passed to mdb_cursor_get.

This commit addresses this lingering case by special-casing return value
processing for the Set operation of Cursor.Get.  If LMDB will not modify
the MDB_val, the returned key must use non-standard methods for copying
the value.  Likewise, when txn.RawRead is true the Cursor.Get must use a
special method to ensure that the returned key shares memory with the
corresponding input argument.
bmatsuo added a commit that referenced this issue Jan 28, 2017
Fixes #96

Issue #56 illustrated that working with Go memory using "C-style" access
patterns does not work well in modern Go runtimes.  That problem was
thought to be properly addressed, but #96 showed a lingering case where
Set does not modify MDB_val key passed to mdb_cursor_get.

This commit addresses this lingering case by special-casing return value
processing for the Set operation of Cursor.Get.  If LMDB will not modify
the MDB_val, the returned key must use non-standard methods for copying
the value.  Likewise, when txn.RawRead is true the Cursor.Get must use a
special method to ensure that the returned key shares memory with the
corresponding input argument.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants