Sitelet https://github.com/erigontech/mdbx-go/commit/3d994538159a7eb30ffa5de4521f71f7bf580861
Skip to content

Commit 3d99453

Browse files
AskAlexSharovAskAlexSharov
authored
backport(v0.41.2): mdbx: bar copying Cursor and Txn with noCopy (#251)
* mdbx: bar copying a Cursor with noCopy A Cursor owns a raw libmdbx cursor, so copying the struct hands two values one underlying cursor: Close on either frees it while the other still points at it, and the second Close is a double free into cgo. Nothing caught that — the struct holds no sync type, so copylocks had nothing to key on. Embedding a Cursor by value, which Open exists for, is unaffected. Zero-sized and placed first, so sizeof(Cursor) is unchanged at 16 bytes. (cherry picked from commit daf9fff) * mdbx: extend noCopy to Txn, move it to its own file Txn owns a raw libmdbx transaction, so a copy hands two values one txn and Abort/Commit on either leaves the other dangling — the same hazard as Cursor. Env needs nothing: its closeLock already makes copylocks reject copies. noCopy moves to nocopy.go so it sits between neither type and its doc comment. (cherry picked from commit a29acaf)
1 parent afa609b commit 3d99453

4 files changed

Lines changed: 34 additions & 2 deletions

File tree

‎mdbx/cursor.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,9 @@ const (
9999
//
100100
// See MDB_cursor.
101101
type Cursor struct {
102-
txn *Txn
103-
_c *C.MDBX_cursor
102+
noCopy noCopy
103+
txn *Txn
104+
_c *C.MDBX_cursor
104105
}
105106

106107
// Open binds an unopened Cursor to the table in place. Unlike Txn.OpenCursor it

‎mdbx/cursor_lifecycle_test.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,3 +287,15 @@ func TestCursorOpenNilArgs(t *testing.T) {
287287
t.Fatal("a failed Open must leave the cursor closed")
288288
}
289289
}
290+
291+
// Embedding a Cursor by value is the pattern Open exists for, so noCopy must
292+
// not stand in its way — it bars copying the Cursor, not holding one.
293+
func TestCursorEmbedByValue(t *testing.T) {
294+
type holder struct{ c Cursor }
295+
296+
h := &holder{}
297+
if !h.c.IsClosed() {
298+
t.Fatal("a zero embedded Cursor must report closed")
299+
}
300+
h.c.Close() // no-op on a never-opened cursor
301+
}

‎mdbx/nocopy.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package mdbx
2+
3+
// noCopy makes `go vet -copylocks` reject copies of the type embedding it.
4+
// The C-handle types here own a raw libmdbx object, so a copy would give two
5+
// values one underlying handle: closing either frees it while the other still
6+
// points at it.
7+
//
8+
// Zero-sized, so it costs no space when placed first in a struct. Held as a
9+
// named field rather than embedded, which would promote Lock/Unlock onto the
10+
// enclosing type's method set.
11+
type noCopy struct{}
12+
13+
func (*noCopy) Lock() {}
14+
func (*noCopy) Unlock() {}

‎mdbx/txn.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ const (
6060
//
6161
// See MDBX_txn.
6262
type Txn struct {
63+
// noCopy: a Txn owns a raw libmdbx transaction, so a copy would give two
64+
// values one underlying txn — Abort or Commit on either leaves the other
65+
// pointing at freed memory. Env is already covered by its closeLock.
66+
noCopy noCopy
67+
6368
env *Env
6469
_txn *C.MDBX_txn
6570
// val is scratch space for Txn.Get and the PutReserve paths: passing the

0 commit comments

Comments
 (0)