Skip to content

Commit

Permalink
MDBX bindings: remove finalizers (write tx require to be closed from …
Browse files Browse the repository at this point in the history
…same thread) (#1579)
  • Loading branch information
AskAlexSharov committed Apr 6, 2021
1 parent 384290c commit a045848
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 48 deletions.
2 changes: 0 additions & 2 deletions ethdb/mdbx/cursor.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package mdbx
*/
import "C"
import (
"runtime"
"unsafe"
)

Expand Down Expand Up @@ -106,7 +105,6 @@ func (c *Cursor) close() bool {
// See mdb_cursor_close.
func (c *Cursor) Close() {
if c.close() {
runtime.SetFinalizer(c, nil)
}
}

Expand Down
11 changes: 2 additions & 9 deletions ethdb/mdbx/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -540,19 +540,12 @@ func (env *Env) SetMaxDBs(size int) error {
// methods, which assist in management of Txn objects and provide OS thread
// locking required for write transactions.
//
// A finalizer detects unreachable, live transactions and logs thems to
// standard error. The transactions are aborted, but their presence should be
// interpreted as an application error which should be patched so transactions
// are terminated explicitly. Unterminated transactions can adversly effect
// Unterminated transactions can adversly effect
// database performance and cause the database to grow until the map is full.
//
// See mdbx_txn_begin.
func (env *Env) BeginTxn(parent *Txn, flags uint) (*Txn, error) {
txn, err := beginTxn(env, parent, flags)
if txn != nil {
runtime.SetFinalizer(txn, func(v interface{}) { v.(*Txn).finalize() })
}
return txn, err
return beginTxn(env, parent, flags)
}

// RunTxn creates a new Txn and calls fn with it as an argument. Run commits
Expand Down
4 changes: 0 additions & 4 deletions ethdb/mdbx/txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import "C"

import (
"log"
"runtime"
"time"
"unsafe"
)
Expand Down Expand Up @@ -196,7 +195,6 @@ func (txn *Txn) Commit() (CommitLatency, error) {
panic("managed transaction cannot be committed directly")
}

runtime.SetFinalizer(txn, nil)
return txn.commit()
}

Expand Down Expand Up @@ -242,7 +240,6 @@ func (txn *Txn) Abort() {
panic("managed transaction cannot be aborted directly")
}

runtime.SetFinalizer(txn, nil)
txn.abort()
}

Expand Down Expand Up @@ -615,7 +612,6 @@ func (txn *Txn) Del(dbi DBI, key, val []byte) error {
func (txn *Txn) OpenCursor(dbi DBI) (*Cursor, error) {
cur, err := openCursor(txn, dbi)
if cur != nil && txn.readonly {
runtime.SetFinalizer(cur, (*Cursor).close)
}
return cur, err
}
Expand Down
33 changes: 0 additions & 33 deletions ethdb/mdbx/txn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"runtime"
"syscall"
"testing"
"time"
)

func TestTxn_ID(t *testing.T) {
Expand Down Expand Up @@ -100,7 +99,6 @@ func TestTxn_ID(t *testing.T) {
}

func TestTxn_errLogf(t *testing.T) {
t.Skip("to investigate")
env := setup(t)
defer clean(env, t)

Expand All @@ -116,37 +114,6 @@ func TestTxn_errLogf(t *testing.T) {
}
}

func TestTxn_finalizer(t *testing.T) {
env := setup(t)
defer clean(env, t)

runtime.LockOSThread()
defer runtime.UnlockOSThread()

called := make(chan struct{})
func() {
txn, err := env.BeginTxn(nil, 0)
if err != nil {
t.Error(err)
} else {
txn.errLogf = func(string, ...interface{}) {
close(called)
}
}
}()

// make sure that finalizer has a chance to get called. it seems like this
// may not be consistent across versions of go.
runtime.GC()
runtime.Gosched()

select {
case <-called:
case <-time.After(time.Second):
t.Errorf("error logging function was not called")
}
}

func TestTxn_Drop(t *testing.T) {
env := setup(t)
defer clean(env, t)
Expand Down

0 comments on commit a045848

Please sign in to comment.