From e24c488fee545fbf704c218306a3053795262551 Mon Sep 17 00:00:00 2001
From: Faruk Kasumovic <faruk@bicomsystems.com>
Date: Wed, 12 Feb 2020 08:35:49 +0100
Subject: [PATCH] - Fix: Concurrent access to properties even for a different
 pools still cause crash

Prevent concurrent access to all properties with global mutex
---
 zfs.go      | 30 +++++++++++-------------------
 zfs_test.go |  6 ++++--
 2 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/zfs.go b/zfs.go
index 7989a35..6b6a013 100644
--- a/zfs.go
+++ b/zfs.go
@@ -311,6 +311,8 @@ func (d *Dataset) PoolName() string {
 
 // ReloadProperties re-read dataset's properties
 func (d *Dataset) ReloadProperties() (err error) {
+	Global.Mtx.Lock()
+	defer Global.Mtx.Unlock()
 	if d.list == nil {
 		err = errors.New(msgDatasetIsNil)
 		return
@@ -318,15 +320,7 @@ func (d *Dataset) ReloadProperties() (err error) {
 	d.Properties = make(map[Prop]Property)
 	C.zfs_refresh_properties(d.list.zh)
 	for prop := DatasetPropType; prop < DatasetNumProps; 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))
-		}
+		plist := C.read_dataset_property(d.list, C.int(prop))
 		if plist == nil {
 			continue
 		}
@@ -340,15 +334,12 @@ func (d *Dataset) ReloadProperties() (err error) {
 // GetProperty reload and return single specified property. This also reloads requested
 // property in Properties map.
 func (d *Dataset) GetProperty(p Prop) (prop Property, err error) {
+	Global.Mtx.Lock()
+	defer Global.Mtx.Unlock()
 	if d.list == nil {
 		err = errors.New(msgDatasetIsNil)
 		return
 	}
-	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()
@@ -363,6 +354,8 @@ func (d *Dataset) GetProperty(p Prop) (prop Property, err error) {
 
 // GetUserProperty - lookup and return user propery
 func (d *Dataset) GetUserProperty(p string) (prop Property, err error) {
+	Global.Mtx.Lock()
+	defer Global.Mtx.Unlock()
 	if d.list == nil {
 		err = errors.New(msgDatasetIsNil)
 		return
@@ -384,16 +377,13 @@ func (d *Dataset) GetUserProperty(p string) (prop Property, err error) {
 // some can be set only at creation time and some are read only.
 // Always check if returned error and its description.
 func (d *Dataset) SetProperty(p Prop, value string) (err error) {
+	Global.Mtx.Lock()
+	defer Global.Mtx.Unlock()
 	if d.list == nil {
 		err = errors.New(msgDatasetIsNil)
 		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 {
@@ -414,6 +404,8 @@ func (d *Dataset) SetProperty(p Prop, value string) (err error) {
 
 // SetUserProperty -
 func (d *Dataset) SetUserProperty(prop, value string) (err error) {
+	Global.Mtx.Lock()
+	defer Global.Mtx.Unlock()
 	if d.list == nil {
 		err = errors.New(msgDatasetIsNil)
 		return
diff --git a/zfs_test.go b/zfs_test.go
index b94fbc8..aeef553 100644
--- a/zfs_test.go
+++ b/zfs_test.go
@@ -214,7 +214,7 @@ func zfsTestMountPointConcurrency(t *testing.T) {
 	gr1 := make(chan bool)
 	gr2 := make(chan bool)
 	go func() {
-		for i := 0; i < 30; i++ {
+		for i := 0; i < 100; i++ {
 			println("reload properties:", i)
 			// d.SetProperty(zfs.DatasetPropMountpoint, "/TEST")
 			d.ReloadProperties()
@@ -224,11 +224,13 @@ func zfsTestMountPointConcurrency(t *testing.T) {
 	go func() {
 		for i := 0; i < 100; i++ {
 			println("set mountpoint:", i)
-			d.SetProperty(zfs.DatasetPropMountpoint, "/TEST")
+			d.ReloadProperties()
+			// d.SetProperty(zfs.DatasetPropMountpoint, "/TEST")
 			// d.GetProperty(zfs.DatasetPropMountpoint)
 		}
 		gr2 <- true
 	}()
+
 	d.SetProperty(zfs.DatasetPropMountpoint, "none")
 
 	<-gr1