mpt: minor doc updates and bug fixes

Address issues identified by a code review from an LLM.

Author: rsc

mpt/disk.go

@@ -84,8 +84,13 @@ var _ pmem.File = File(nil)
 // A diskTree is an on-disk [Tree].
 type diskTree struct {
 	// mmu is the memory mapping mutex.
+	//
 	// All methods except Close do mmu.RLock and mmu.RUnlock
-	// in order to be allowed to use pmem.Data() aka mem.
+	// in order to be allowed to read and write pmem.Data() aka mem.
+	// Note that it is OK to write the memory while holding the "RLock".
+	// The point of the shared RLock is to stop Close from unmapping
+	// the memory entirely.
+	//
 	// Close calls mmu.Lock/mmu.Unlock to wait for all other
 	// method calls to finish before unmapping the memory.
 	mmu  sync.RWMutex
@@ -245,8 +250,9 @@ func (t *diskTree) Close() error {
 	if t.closed {
 		return fmt.Errorf("tree already closed")
 	}
+	t.closed = true
 	if err := t.pmem.Sync(); err != nil {
-		return t.broken(err)
+		t.broken(err)
 	}
 	if err := t.pmem.Release(); err != nil {
 		t.broken(err)
@@ -262,10 +268,12 @@ func (t *diskTree) Close() error {
 	if err := t.file2.Close(); err != nil {
 		t.broken(err)
 	}
+	if err := t.leaf.Close(); err != nil {
+		t.broken(err)
+	}
 	if t.err != nil {
 		return t.err
 	}
-	t.closed = true
 	t.err = errors.New("tree is closed") // stop future method calls
 	return nil
 }

mpt/disk_test.go

@@ -236,7 +236,7 @@ func testDiskRecovery(t *testing.T) {
 		t.Fatal(err)
 	}
 	tree := xtree.(*diskTree)
-	defer tree.Close() // relelase pmem on test failure
+	defer tree.Close() // release pmem on test failure
 
 	tree.pmem.SetConstantFlushing(true)
 	tt.tree = tree

mpt/dmem.go

@@ -88,6 +88,8 @@ func (t *diskTree) snap(version int64) error {
 		// updating arbitrarily many hashes during rehash.
 		// Without group, ordering matters: write hash before dirty
 		// and both before version.
+		//
+		// Also note: dirty implies that tree is non-empty, so there is a root.
 		root, err := t.node(t.hdr().root())
 		if err != nil {
 			return err
@@ -113,6 +115,9 @@ func (t *diskTree) snap(version int64) error {
 
 // Version returns version information about the tree.
 func (t *diskTree) Version() (version int64, exact bool) {
+	t.mmu.RLock()
+	defer t.mmu.RUnlock()
+
 	hdr := t.hdr()
 	return hdr.version(), hdr.exact()
 }

mpt/internal/slicemath/slicemath.go

@@ -7,14 +7,15 @@ import "unsafe"
 
 // contains reports whether big contains little;
 // that is, it reports whether little is a subslice of big.
+// If either big or little is empty, contains returns false.
 func contains(big, little []byte) bool {
-	return uintptr(unsafe.Pointer(&big[0])) <= uintptr(unsafe.Pointer(&little[0])) &&
+	return len(big) > 0 && len(little) > 0 &&
+		uintptr(unsafe.Pointer(&big[0])) <= uintptr(unsafe.Pointer(&little[0])) &&
 		uintptr(unsafe.Pointer(&little[len(little)-1])) <= uintptr(unsafe.Pointer(&big[len(big)-1]))
 }
 
 // Offset reports little's starting position within big.
 // If big does not contain little, Offset returns ^uintptr(0), false.
-// The caller must have checked sliceContains(big, little) already.
 func Offset(big, little []byte) (offset uintptr, ok bool) {
 	if !contains(big, little) {
 		return ^uintptr(0), false

mpt/tree.go

@@ -67,6 +67,9 @@ type Tree interface {
 	// to flush the changes to disk. (If the files are *os.File files,
 	// Sync calls fsync(2).)
 	//
+	// Sync must not be called concurrently with other calls to Sync
+	// or (as noted above) with calls to Set.
+	//
 	// Even in the absence of calls to Sync, a Tree provides the
 	// guarantee that on recovery from a crash, it can identify the
 	// latest snapshot whose Set calls are fully included in the tree.
@@ -366,11 +369,7 @@ func hashInner(b int, left, right Hash) Hash {
 	copy(enc[:32], left[:])
 	copy(enc[32:64], right[:])
 	enc[64] = byte(b)
-	h := sha256.Sum256(enc[:])
-	if right == (Hash{}) {
-		panic("zero")
-	}
-	return h
+	return sha256.Sum256(enc[:])
 }
 
 func reduce(s []node) []node {