diff --git a/lmdb/env.go b/lmdb/env.go index 4d10416..7e68ed7 100644 --- a/lmdb/env.go +++ b/lmdb/env.go @@ -113,13 +113,13 @@ func (env *Env) FD() (uintptr, error) { // to avoid constant value overflow errors at compile time. const fdInvalid = ^uintptr(0) - mf := new(C.mdb_filehandle_t) - ret := C.mdb_env_get_fd(env._env, mf) + var mf C.mdb_filehandle_t + ret := C.mdb_env_get_fd(env._env, &mf) err := operrno("mdb_env_get_fd", ret) if err != nil { return 0, err } - fd := uintptr(*mf) + fd := uintptr(mf) if fd == fdInvalid { return 0, errNotOpen diff --git a/lmdb/txn.go b/lmdb/txn.go index 1d68b60..6505e72 100644 --- a/lmdb/txn.go +++ b/lmdb/txn.go @@ -63,6 +63,9 @@ type Txn struct { // reset/renewed id uintptr + // Pointer to scratch space for key and val in readonly transactions + cbuf unsafe.Pointer + env *Env _txn *C.MDB_txn key *C.MDB_val @@ -87,12 +90,20 @@ func beginTxn(env *Env, parent *Txn, flags uint) (*Txn, error) { txn.key = env.ckey txn.val = env.cval } else { - // It is not easy to share C.MDB_val values in this scenario unless - // there is a synchronized pool involved, which will increase - // overhead. Further, allocating these values with C will add - // overhead both here and when the values are freed. - txn.key = new(C.MDB_val) - txn.val = new(C.MDB_val) + // Readonly transaction. + // We cannot share global C.MDB_val values in this scenario, because + // there can be multiple simultaneous read transactions. + // Allocate C memory for two values in one call. + // This is freed in clearTxn(), which is always called at the end + // of a transaction through Commit() or Abort(). + if C.sizeof_MDB_val == 0 { + panic("zero C.sizeof_MDB_val") // should never happen + } + txn.cbuf = C.malloc(2 * C.sizeof_MDB_val) + txn.key = (*C.MDB_val)(txn.cbuf) + txn.val = (*C.MDB_val)(unsafe.Pointer( + uintptr(txn.cbuf) + uintptr(C.sizeof_MDB_val), + )) } } else { // Because parent Txn objects cannot be used while a sub-Txn is active @@ -240,6 +251,14 @@ func (txn *Txn) clearTxn() { // sure the value returned for an invalid Txn is more or less consistent // for people familiar with the C semantics. txn.resetID() + + // Release C allocated buffer, if used + if txn.cbuf != nil { + txn.key = nil + txn.val = nil + C.free(txn.cbuf) + txn.cbuf = nil + } } // resetID has to be called anytime the value of Txn.getID() may change diff --git a/lmdb/txn_test.go b/lmdb/txn_test.go index 45274e8..9abf890 100644 --- a/lmdb/txn_test.go +++ b/lmdb/txn_test.go @@ -918,6 +918,69 @@ func TestTxn_Reset_writeTxn(t *testing.T) { } } +// This test demonstrates that in a readonly transaction C memory is allocated +// for the values, and freed during a Reset. +func TestTxn_Reset_readonly_C_free(t *testing.T) { + env := setup(t) + path, err := env.Path() + if err != nil { + env.Close() + t.Error(err) + return + } + defer os.RemoveAll(path) + defer env.Close() + + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + txn, err := env.BeginTxn(nil, Readonly) + if err != nil { + t.Error(err) + return + } + defer txn.Abort() + + // Since this is a readonly transaction, the global Env key/val cannot be + // reused and new C memory must be allocated. + if txn.key == env.ckey || txn.val == env.cval { + t.Error("env.ckey and cval must not be used in a readonly Txn") + } + if txn.cbuf == nil { + t.Error("cbuf expected to not be nil when opening a readonly Txn") + } + + // Reset must not free the buffer + txn.Reset() + if txn.cbuf == nil { + t.Error("cbuf must not be nil after Reset") + return + } + + // Renew must not free the buffer + err = txn.Renew() + if err != nil { + t.Error(err) + return + } + if txn.cbuf == nil { + t.Error("cbuf must not be nil after Renew") + return + } + + // Abort must free the buffer + txn.Abort() + if txn.cbuf != nil { + t.Error("cbuf expected to be nil, C memory not freed") + } + if txn.key != nil || txn.val != nil { + t.Error("key and val expected to be nil after C memory free") + } + + // A second Abort must not panic + txn.Abort() +} + func TestTxn_UpdateLocked(t *testing.T) { env := setup(t) path, err := env.Path()