From 7c7ef79611932db89bad0f9c653e7b87155958d3 Mon Sep 17 00:00:00 2001
From: Faruk Kasumovic <faruk@bicomsystems.com>
Date: Mon, 10 Feb 2020 11:12:46 +0100
Subject: [PATCH] - Fix mount point property race condition causing
 segmentation fault failures

---
 a_test.go   |  3 +++
 zfs.go      | 34 ++++++++++++++++++++++++++--------
 zfs_test.go | 34 +++++++++++++++++++++++++++++++++-
 3 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/a_test.go b/a_test.go
index 5a663ec..ab32e3b 100644
--- a/a_test.go
+++ b/a_test.go
@@ -22,6 +22,9 @@ func Test(t *testing.T) {
 
 	zfsTestDatasetCreate(t)
 	zfsTestDatasetOpen(t)
+	// zfsTestMountPointConcurrency(t)
+	// time.Sleep(15 * time.Second)
+
 	zfsTestDatasetSnapshot(t)
 	zfsTestDatasetOpenAll(t)
 	zfsTestDatasetSetProperty(t)
diff --git a/zfs.go b/zfs.go
index af3f32d..7989a35 100644
--- a/zfs.go
+++ b/zfs.go
@@ -125,13 +125,12 @@ func DatasetOpenSingle(path string) (d Dataset, err error) {
 	if d.list == nil || d.list.zh == nil {
 		err = LastError()
 		if err == nil {
-			err = fmt.Errorf("dataset not found.")
+			err = fmt.Errorf("dataset not found")
 		}
 		err = fmt.Errorf("%s - %s", err.Error(), path)
 		return
 	}
 	d.Type = DatasetType(C.dataset_type(d.list))
-	d.Properties = make(map[Prop]Property)
 	err = d.ReloadProperties()
 	if err != nil {
 		return
@@ -317,11 +316,17 @@ func (d *Dataset) ReloadProperties() (err error) {
 		return
 	}
 	d.Properties = make(map[Prop]Property)
-	Global.Mtx.Lock()
-	defer Global.Mtx.Unlock()
 	C.zfs_refresh_properties(d.list.zh)
 	for prop := DatasetPropType; prop < DatasetNumProps; prop++ {
-		plist := C.read_dataset_property(d.list, C.int(prop))
+		var plist *C.property_list_t
+		if prop == DatasetPropMounted || prop == DatasetPropMountpoint {
+			// prevent zfs mountpoint race conditions
+			Global.Mtx.Lock()
+			plist = C.read_dataset_property(d.list, C.int(prop))
+			Global.Mtx.Unlock()
+		} else {
+			plist = C.read_dataset_property(d.list, C.int(prop))
+		}
 		if plist == nil {
 			continue
 		}
@@ -339,8 +344,11 @@ func (d *Dataset) GetProperty(p Prop) (prop Property, err error) {
 		err = errors.New(msgDatasetIsNil)
 		return
 	}
-	Global.Mtx.Lock()
-	defer Global.Mtx.Unlock()
+	if p == DatasetPropMounted || p == DatasetPropMountpoint {
+		// prevent zfs mountpoint race conditions
+		Global.Mtx.Lock()
+		defer Global.Mtx.Unlock()
+	}
 	plist := C.read_dataset_property(d.list, C.int(p))
 	if plist == nil {
 		err = LastError()
@@ -381,6 +389,11 @@ func (d *Dataset) SetProperty(p Prop, value string) (err error) {
 		return
 	}
 	csValue := C.CString(value)
+	if p == DatasetPropMounted || p == DatasetPropMountpoint {
+		// prevent zfs mountpoint race conditions
+		Global.Mtx.Lock()
+		defer Global.Mtx.Unlock()
+	}
 	errcode := C.dataset_prop_set(d.list, C.zfs_prop_t(p), csValue)
 	C.free(unsafe.Pointer(csValue))
 	if errcode != 0 {
@@ -388,9 +401,14 @@ func (d *Dataset) SetProperty(p Prop, value string) (err error) {
 		return
 	}
 	// Update Properties member with change made
-	if _, err = d.GetProperty(p); err != nil {
+	plist := C.read_dataset_property(d.list, C.int(p))
+	if plist == nil {
+		err = LastError()
 		return
 	}
+	defer C.free_properties(plist)
+	d.Properties[p] = Property{Value: C.GoString(&(*plist).value[0]),
+		Source: C.GoString(&(*plist).source[0])}
 	return
 }
 
diff --git a/zfs_test.go b/zfs_test.go
index dbd9ea0..b94fbc8 100644
--- a/zfs_test.go
+++ b/zfs_test.go
@@ -4,7 +4,7 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/bicomsystems/go-libzfs"
+	zfs "github.com/bicomsystems/go-libzfs"
 )
 
 /* ------------------------------------------------------------------------- */
@@ -203,6 +203,38 @@ func zfsTestDatasetDestroy(t *testing.T) {
 	print("PASS\n\n")
 }
 
+func zfsTestMountPointConcurrency(t *testing.T) {
+	println("TEST DATASET MountPointConcurrency( ", TSTDatasetPath, " ) ... ")
+	d, err := zfs.DatasetOpen(TSTDatasetPath)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+	defer d.Close()
+	gr1 := make(chan bool)
+	gr2 := make(chan bool)
+	go func() {
+		for i := 0; i < 30; i++ {
+			println("reload properties:", i)
+			// d.SetProperty(zfs.DatasetPropMountpoint, "/TEST")
+			d.ReloadProperties()
+		}
+		gr1 <- true
+	}()
+	go func() {
+		for i := 0; i < 100; i++ {
+			println("set mountpoint:", i)
+			d.SetProperty(zfs.DatasetPropMountpoint, "/TEST")
+			// d.GetProperty(zfs.DatasetPropMountpoint)
+		}
+		gr2 <- true
+	}()
+	d.SetProperty(zfs.DatasetPropMountpoint, "none")
+
+	<-gr1
+	<-gr2
+}
+
 /* ------------------------------------------------------------------------- */
 // EXAMPLES: