Fix MapOf struct value pointer uniqueness (#47)

Also fixes nil value uniquness in Map

export_test.go

@@ -29,3 +29,11 @@ func StoreTopHash(hash, topHashes uint64, idx int) uint64 {
 func EraseTopHash(topHashes uint64, idx int) uint64 {
 	return eraseTopHash(topHashes, idx)
 }
+
+func EnableAssertions() {
+	assertionsEnabled = true
+}
+
+func DisableAssertions() {
+	assertionsEnabled = false
+}

map.go

@@ -87,11 +87,6 @@ type rangeEntry struct {
 	value unsafe.Pointer
 }
 
-// special type to mark nil values
-type nilValue struct{}
-
-var nilVal = new(nilValue)
-
 // NewMap creates a new Map instance.
 func NewMap() *Map {
 	m := &Map{}
@@ -169,9 +164,6 @@ func (m *Map) LoadAndStore(key string, value interface{}) (actual interface{}, l
 }
 
 func (m *Map) doStore(key string, value interface{}, loadIfExists bool) (interface{}, bool) {
-	if value == nil {
-		value = nilVal
-	}
 	// Read-only path.
 	if loadIfExists {
 		if v, ok := m.Load(key); ok {
@@ -222,7 +214,11 @@ func (m *Map) doStore(key string, value interface{}, loadIfExists bool) (interfa
 				// interface{} on each call, thus the live value pointers are
 				// unique. Otherwise atomic snapshot won't be correct in case
 				// of multiple Store calls using the same value.
-				atomic.StorePointer(&b.values[i], unsafe.Pointer(&value))
+				nvp := unsafe.Pointer(&value)
+				if assertionsEnabled && vp == nvp {
+					panic("non-unique value pointer")
+				}
+				atomic.StorePointer(&b.values[i], nvp)
 				b.mu.Unlock()
 				return derefValue(vp), true
 			}
@@ -477,11 +473,7 @@ func derefKey(keyPtr unsafe.Pointer) string {
 }
 
 func derefValue(valuePtr unsafe.Pointer) interface{} {
-	value := *(*interface{})(valuePtr)
-	if _, ok := value.(*nilValue); ok {
-		return nil
-	}
-	return value
+	return *(*interface{})(valuePtr)
 }
 
 func bucketIdx(table *mapTable, hash uint64) uint64 {

map_test.go

@@ -50,6 +50,61 @@ func TestMap_BucketStructSize(t *testing.T) {
 	}
 }
 
+func TestMap_UniqueValuePointers_Int(t *testing.T) {
+	EnableAssertions()
+	m := NewMap()
+	v := 42
+	m.Store("foo", v)
+	m.Store("foo", v)
+	DisableAssertions()
+}
+
+func TestMap_UniqueValuePointers_Struct(t *testing.T) {
+	type foo struct{}
+	EnableAssertions()
+	m := NewMap()
+	v := foo{}
+	m.Store("foo", v)
+	m.Store("foo", v)
+	DisableAssertions()
+}
+
+func TestMap_UniqueValuePointers_Pointer(t *testing.T) {
+	type foo struct{}
+	EnableAssertions()
+	m := NewMap()
+	v := &foo{}
+	m.Store("foo", v)
+	m.Store("foo", v)
+	DisableAssertions()
+}
+
+func TestMap_UniqueValuePointers_Slice(t *testing.T) {
+	EnableAssertions()
+	m := NewMap()
+	v := make([]int, 13)
+	m.Store("foo", v)
+	m.Store("foo", v)
+	DisableAssertions()
+}
+
+func TestMap_UniqueValuePointers_String(t *testing.T) {
+	EnableAssertions()
+	m := NewMap()
+	v := "bar"
+	m.Store("foo", v)
+	m.Store("foo", v)
+	DisableAssertions()
+}
+
+func TestMap_UniqueValuePointers_Nil(t *testing.T) {
+	EnableAssertions()
+	m := NewMap()
+	m.Store("foo", nil)
+	m.Store("foo", nil)
+	DisableAssertions()
+}
+
 func TestMap_MissingEntry(t *testing.T) {
 	m := NewMap()
 	v, ok := m.Load("foo")

mapof.go

@@ -146,11 +146,16 @@ func (m *MapOf[V]) doStore(key string, value V, loadIfExists bool) (V, bool) {
 					b.mu.Unlock()
 					return derefTypedValue[V](vp), true
 				}
-				// In-place update case. Luckily we get a copy of the value
+				// In-place update case. We get a copy of the value via an
 				// interface{} on each call, thus the live value pointers are
 				// unique. Otherwise atomic snapshot won't be correct in case
 				// of multiple Store calls using the same value.
-				atomic.StorePointer(&b.values[i], unsafe.Pointer(&value))
+				var wv interface{} = value
+				nvp := unsafe.Pointer(&wv)
+				if assertionsEnabled && vp == nvp {
+					panic("non-unique value pointer")
+				}
+				atomic.StorePointer(&b.values[i], nvp)
 				b.mu.Unlock()
 				return derefTypedValue[V](vp), true
 			}
@@ -159,7 +164,8 @@ func (m *MapOf[V]) doStore(key string, value V, loadIfExists bool) (V, bool) {
 			// Insertion case. First we update the value, then the key.
 			// This is important for atomic snapshot states.
 			atomic.StoreUint64(&b.topHashes, storeTopHash(hash, b.topHashes, emptyidx))
-			atomic.StorePointer(emptyvp, unsafe.Pointer(&value))
+			var wv interface{} = value
+			atomic.StorePointer(emptyvp, unsafe.Pointer(&wv))
 			atomic.StorePointer(emptykp, unsafe.Pointer(&key))
 			b.mu.Unlock()
 			addSize(table, bidx, 1)
@@ -336,7 +342,7 @@ func (m *MapOf[V]) Size() int {
 }
 
 func derefTypedValue[V any](valuePtr unsafe.Pointer) (val V) {
-	return *(*V)(valuePtr)
+	return (*(*interface{})(valuePtr)).(V)
 }
 
 // O(N) operation; use for debug purposes only

mapof_test.go

@@ -12,6 +12,61 @@ import (
 	. "github.com/puzpuzpuz/xsync"
 )
 
+func TestMapOf_UniqueValuePointers_Int(t *testing.T) {
+	EnableAssertions()
+	m := NewMapOf[int]()
+	v := 42
+	m.Store("foo", v)
+	m.Store("foo", v)
+	DisableAssertions()
+}
+
+func TestMapOf_UniqueValuePointers_Struct(t *testing.T) {
+	type foo struct{}
+	EnableAssertions()
+	m := NewMapOf[foo]()
+	v := foo{}
+	m.Store("foo", v)
+	m.Store("foo", v)
+	DisableAssertions()
+}
+
+func TestMapOf_UniqueValuePointers_Pointer(t *testing.T) {
+	type foo struct{}
+	EnableAssertions()
+	m := NewMapOf[*foo]()
+	v := &foo{}
+	m.Store("foo", v)
+	m.Store("foo", v)
+	DisableAssertions()
+}
+
+func TestMapOf_UniqueValuePointers_Slice(t *testing.T) {
+	EnableAssertions()
+	m := NewMapOf[[]int]()
+	v := make([]int, 13)
+	m.Store("foo", v)
+	m.Store("foo", v)
+	DisableAssertions()
+}
+
+func TestMapOf_UniqueValuePointers_String(t *testing.T) {
+	EnableAssertions()
+	m := NewMapOf[string]()
+	v := "bar"
+	m.Store("foo", v)
+	m.Store("foo", v)
+	DisableAssertions()
+}
+
+func TestMapOf_UniqueValuePointers_Nil(t *testing.T) {
+	EnableAssertions()
+	m := NewMapOf[*struct{}]()
+	m.Store("foo", nil)
+	m.Store("foo", nil)
+	DisableAssertions()
+}
+
 func TestMapOf_MissingEntry(t *testing.T) {
 	m := NewMapOf[string]()
 	v, ok := m.Load("foo")

util.go

@@ -5,6 +5,9 @@ import (
 	"unsafe"
 )
 
+// test-only assert()-like flag
+var assertionsEnabled = false
+
 const (
 	// used in paddings to prevent false sharing;
 	// 64B are used instead of 128B as a compromise between