Fixing 64-bit alignment in structs that use sync.atomic

This change fixes segmentation faults on Arm v7.

From https://golang.org/pkg/sync/atomic/:

> On both ARM and x86-32, it is the caller's responsibility to arrange
for 64-bit alignment of 64-bit words accessed atomically. The first word
in a variable or in an allocated struct, array, or slice can be relied
upon to be 64-bit aligned.

Had to modify a couple of tests to get them to pass.

Fixes #311.

Author: deepakjois

db_test.go

@@ -328,7 +328,7 @@ func TestGetMore(t *testing.T) {
 	}
 	//	n := 500000
 	n := 10000
-	m := 49 // Increasing would cause ErrTxnTooBig
+	m := 45 // Increasing would cause ErrTxnTooBig
 	for i := 0; i < n; i += m {
 		txn := kv.NewTransaction(true)
 		for j := i; j < i+m && j < n; j++ {
@@ -432,7 +432,7 @@ func TestExistsMore(t *testing.T) {
 
 	//	n := 500000
 	n := 10000
-	m := 49
+	m := 45
 	for i := 0; i < n; i += m {
 		if (i % 1000) == 0 {
 			t.Logf("Putting i=%d\n", i)

levels.go

@@ -32,14 +32,13 @@ import (
 )
 
 type levelsController struct {
-	elog trace.EventLog
+	nextFileID uint64 // Atomic
+	elog       trace.EventLog
 
 	// The following are initialized once and const.
 	levels []*levelHandler
 	kv     *DB
 
-	nextFileID uint64 // Atomic
-
 	cstatus compactStatus
 }
 

skl/skl.go

@@ -50,19 +50,20 @@ const (
 const MaxNodeSize = int(unsafe.Sizeof(node{}))
 
 type node struct {
+	// Multiple parts of the value are encoded as a single uint64 so that it
+	// can be atomically loaded and stored:
+	//   value offset: uint32 (bits 0-31)
+	//   value size  : uint16 (bits 32-47)
+	// 12 bytes are allocated to ensure 8 byte alignment also on 32bit systems.
+	value [12]byte
+
 	// A byte slice is 24 bytes. We are trying to save space here.
 	keyOffset uint32 // Immutable. No need to lock to access key.
 	keySize   uint16 // Immutable. No need to lock to access key.
 
 	// Height of the tower.
 	height uint16
 
-	// Multiple parts of the value are encoded as a single uint64 so that it
-	// can be atomically loaded and stored:
-	//   value offset: uint32 (bits 0-31)
-	//   value size  : uint16 (bits 32-47)
-	value uint64
-
 	// Most nodes do not need to use the full height of the tower, since the
 	// probability of each successive level decreases exponentially. Because
 	// these elements are never accessed, they do not need to be allocated.
@@ -108,7 +109,7 @@ func newNode(arena *Arena, key []byte, v y.ValueStruct, height int) *node {
 	node.keyOffset = arena.putKey(key)
 	node.keySize = uint16(len(key))
 	node.height = uint16(height)
-	node.value = encodeValue(arena.putVal(v), v.EncodedSize())
+	*node.value64BitAlignedPtr() = encodeValue(arena.putVal(v), v.EncodedSize())
 	return node
 }
 
@@ -134,8 +135,15 @@ func NewSkiplist(arenaSize int64) *Skiplist {
 	}
 }
 
+func (s *node) value64BitAlignedPtr() *uint64 {
+	if uintptr(unsafe.Pointer(&s.value))%8 == 0 {
+		return (*uint64)(unsafe.Pointer(&s.value))
+	}
+	return (*uint64)(unsafe.Pointer(&s.value[4]))
+}
+
 func (s *node) getValueOffset() (uint32, uint16) {
-	value := atomic.LoadUint64(&s.value)
+	value := atomic.LoadUint64(s.value64BitAlignedPtr())
 	return decodeValue(value)
 }
 
@@ -146,7 +154,7 @@ func (s *node) key(arena *Arena) []byte {
 func (s *node) setValue(arena *Arena, v y.ValueStruct) {
 	valOffset := arena.putVal(v)
 	value := encodeValue(valOffset, v.EncodedSize())
-	atomic.StoreUint64(&s.value, value)
+	atomic.StoreUint64(s.value64BitAlignedPtr(), value)
 }
 
 func (s *node) getNextOffset(h int) uint32 {

transaction.go

@@ -46,10 +46,11 @@ func (u *uint64Heap) Pop() interface{} {
 }
 
 type oracle struct {
+	curRead   uint64 // Managed by the mutex.
+	refCount  int64
 	isManaged bool // Does not change value, so no locking required.
 
 	sync.Mutex
-	curRead    uint64
 	nextCommit uint64
 
 	// These two structures are used to figure out when a commit is done. The minimum done commit is
@@ -59,8 +60,7 @@ type oracle struct {
 
 	// commits stores a key fingerprint and latest commit counter for it.
 	// refCount is used to clear out commits map to avoid a memory blowup.
-	commits  map[uint64]uint64
-	refCount int64
+	commits map[uint64]uint64
 }
 
 func (o *oracle) addRef() {
@@ -192,7 +192,7 @@ type Txn struct {
 func (txn *Txn) checkSize(e *entry) error {
 	count := txn.count + 1
 	// Extra bytes for version in key.
-	size := txn.size + int64(e.estimateSize(txn.db.opt.ValueThreshold)) + 10 
+	size := txn.size + int64(e.estimateSize(txn.db.opt.ValueThreshold)) + 10
 	if count >= txn.db.opt.maxBatchCount || size >= txn.db.opt.maxBatchSize {
 		return ErrTxnTooBig
 	}

transaction_test.go

@@ -23,6 +23,7 @@ import (
 	"strconv"
 	"testing"
 
+	"github.com/dgraph-io/badger/options"
 	"github.com/dgraph-io/badger/y"
 
 	"github.com/stretchr/testify/require"
@@ -617,3 +618,49 @@ func TestManagedDB(t *testing.T) {
 	}
 	txn.Discard()
 }
+
+func TestArmV7Issue311Fix(t *testing.T) {
+	dir, err := ioutil.TempDir("", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	config := DefaultOptions
+	config.TableLoadingMode = options.MemoryMap
+	config.ValueLogFileSize = 16 << 20
+	config.LevelOneSize = 8 << 20
+	config.MaxTableSize = 2 << 20
+	config.Dir = dir
+	config.ValueDir = dir
+	config.SyncWrites = false
+
+	db, err := Open(config)
+	if err != nil {
+		t.Fatalf("cannot open db at location %s: %v", dir, err)
+	}
+
+	err = db.View(func(txn *Txn) error { return nil })
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = db.Update(func(txn *Txn) error {
+		return txn.Set([]byte{0x11}, []byte{0x22})
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = db.Update(func(txn *Txn) error {
+		return txn.Set([]byte{0x11}, []byte{0x22})
+	})
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if err = db.Close(); err != nil {
+		t.Fatal(err)
+	}
+}