- Prevent double-free failure when the same dataset is closed multiple times

In some cases, copying dataset object, either by passing by value or in
some other pattern can lead to code that will close the same dataset
multiple times (e.g. defer). Or otherwise, code with complicated
handling of dataset cleanup.
Fixes: #30, Closes: #31
This commit is contained in:
Faruk Kasumovic 2020-09-09 10:22:37 +02:00
parent 768fbf7f22
commit 039b070d1a
5 changed files with 62 additions and 13 deletions

View File

@ -34,6 +34,7 @@ func Test(t *testing.T) {
zfsTestDatasetSetProperty(t)
zfsTestDatasetHoldRelease(t)
zfsTestDoubleFreeOnDestroy(t)
zfsTestDatasetDestroy(t)
zpoolTestPoolDestroy(t)

View File

@ -60,4 +60,5 @@ func TestDataset_DestroyPromote(t *testing.T) {
t.Log("Destroy promote completed with success")
d.Close()
zpoolTestPoolDestroy(t)
cleanupVDisks()
}

27
zfs.go
View File

@ -13,6 +13,7 @@ import (
"path"
"sort"
"strings"
"sync"
"time"
"unsafe"
)
@ -49,6 +50,7 @@ type HoldTag struct {
// Dataset - ZFS dataset object
type Dataset struct {
list C.dataset_list_ptr
closeOnce *sync.Once
Type DatasetType
Properties map[Prop]Property
Children []Dataset
@ -58,7 +60,7 @@ func (d *Dataset) openChildren() (err error) {
d.Children = make([]Dataset, 0, 5)
list := C.dataset_list_children(d.list)
for list != nil {
dataset := Dataset{list: list}
dataset := Dataset{list: list, closeOnce: new(sync.Once)}
dataset.Type = DatasetType(C.dataset_type(list))
dataset.Properties = make(map[Prop]Property)
err = dataset.ReloadProperties()
@ -79,16 +81,20 @@ func (d *Dataset) openChildren() (err error) {
// DatasetOpenAll recursive get handles to all available datasets on system
// (file-systems, volumes or snapshots).
func DatasetOpenAll() (datasets []Dataset, err error) {
var dataset Dataset
dataset.list = C.dataset_list_root()
for dataset.list != nil {
dataset.Type = DatasetType(C.dataset_type(dataset.list))
list := C.dataset_list_root()
for list != nil {
dataset := Dataset{
list: list,
closeOnce: new(sync.Once),
Type: DatasetType(C.dataset_type(list)),
}
dataset.Type = DatasetType(C.dataset_type(list))
err = dataset.ReloadProperties()
if err != nil {
return
}
datasets = append(datasets, dataset)
dataset.list = C.dataset_next(dataset.list)
list = C.dataset_next(list)
}
for ci := range datasets {
if err = datasets[ci].openChildren(); err != nil {
@ -130,6 +136,7 @@ func DatasetOpenSingle(path string) (d Dataset, err error) {
err = fmt.Errorf("%s - %s", err.Error(), path)
return
}
d.closeOnce = new(sync.Once)
d.Type = DatasetType(C.dataset_type(d.list))
err = d.ReloadProperties()
if err != nil {
@ -182,11 +189,13 @@ func DatasetCreate(path string, dtype DatasetType,
// Close close dataset and all its recursive children datasets (close handle
// and cleanup dataset object/s from memory)
func (d *Dataset) Close() {
// path, _ := d.Path()
Global.Mtx.Lock()
// if dataset was ever open
if d.closeOnce != nil {
d.closeOnce.Do(func() {
C.dataset_list_close(d.list)
})
}
d.list = nil
Global.Mtx.Unlock()
for _, cd := range d.Children {
cd.Close()
}

View File

@ -338,3 +338,35 @@ func ExampleDatasetOpenAll() {
}
}
func CopyAndDestroy(d *zfs.Dataset) (err error) {
if err = d.Destroy(false); err != nil {
return
}
d.Close()
return
}
func zfsTestDoubleFreeOnDestroy(t *testing.T) {
TSTDestroyPath := TSTPoolName + "/DESTROY"
println("TEST Doble Free On Destroy( ", TSTVolumePath, " ) ... ")
props := make(map[zfs.Prop]zfs.Property)
d, err := zfs.DatasetCreate(TSTDestroyPath, zfs.DatasetTypeFilesystem, props)
if err != nil {
t.Error(err)
return
}
d.Close()
d, err = zfs.DatasetOpen(TSTDestroyPath)
if err != nil {
t.Error(err)
return
}
defer d.Close()
if err = CopyAndDestroy(&d); err != nil {
t.Error(err)
return
}
print("PASS\n\n")
}

View File

@ -51,12 +51,18 @@ func createTestpoolVdisks() (err error) {
return
}
func removeVDisk(path string) {
if err := os.Remove(path); err != nil {
println("Error: ", err.Error())
}
}
// Cleanup sparse files used for tests
func cleanupVDisks() {
// try cleanup
os.Remove(s1path)
os.Remove(s2path)
os.Remove(s3path)
removeVDisk(s1path)
removeVDisk(s2path)
removeVDisk(s3path)
}
/* ------------------------------------------------------------------------- */