More file closing (#376)

* file cleanup order

libsq/core/ioz/ioz.go

@@ -45,6 +45,49 @@ func Close(ctx context.Context, c io.Closer) {
 	lg.WarnIfError(log, "Close", err)
 }
 
+// CopyFile copies the contents from src to dst atomically.
+// If dst does not exist, CopyFile creates it with src's perms.
+// If the copy fails, CopyFile aborts and dst is preserved.
+// If mkdir is true, the directory for dst is created if it
+// doesn't exist.
+func CopyFile(dst, src string, mkdir bool) error {
+	if mkdir {
+		if err := RequireDir(filepath.Dir(dst)); err != nil {
+			return err
+		}
+	}
+
+	fi, err := os.Stat(src)
+	if err != nil {
+		return errz.Err(err)
+	}
+
+	in, err := os.Open(src)
+	if err != nil {
+		return errz.Err(err)
+	}
+	defer in.Close()
+	tmp, err := os.CreateTemp(filepath.Dir(dst), "")
+	if err != nil {
+		return errz.Err(err)
+	}
+	_, err = io.Copy(tmp, in)
+	if err != nil {
+		_ = tmp.Close()
+		_ = os.Remove(tmp.Name())
+		return errz.Err(err)
+	}
+	if err = tmp.Close(); err != nil {
+		_ = os.Remove(tmp.Name())
+		return errz.Err(err)
+	}
+	if err = os.Chmod(tmp.Name(), fi.Mode()); err != nil {
+		_ = os.Remove(tmp.Name())
+		return errz.Err(err)
+	}
+	return errz.Err(os.Rename(tmp.Name(), dst))
+}
+
 // CopyAsync asynchronously copies from r to w, invoking
 // non-nil callback when done.
 func CopyAsync(w io.Writer, r io.Reader, callback func(written int64, err error)) {

libsq/driver/grips.go

@@ -32,10 +32,12 @@ type ScratchSrcFunc func(ctx context.Context, name string) (src *source.Source,
 // and then closed by Close. This may be a bad approach.
 type Grips struct {
 	drvrs        Provider
+	closeErr     error
 	scratchSrcFn ScratchSrcFunc
 	files        *files.Files
 	grips        map[string]Grip
 	clnup        *cleanup.Cleanup
+	closeOnce    sync.Once
 	mu           sync.Mutex
 }
 
@@ -417,7 +419,10 @@ func (gs *Grips) OpenJoin(ctx context.Context, srcs ...*source.Source) (Grip, er
 
 // Close closes gs, invoking any cleanup funcs.
 func (gs *Grips) Close() error {
-	return gs.clnup.Run()
+	gs.closeOnce.Do(func() {
+		gs.closeErr = gs.clnup.Run()
+	})
+	return gs.closeErr
 }
 
 var _ Grip = (*cleanOnCloseGrip)(nil)

libsq/files/download.go

@@ -7,15 +7,14 @@ import (
 	"path/filepath"
 	"time"
 
-	"github.com/neilotoole/sq/libsq/core/lg"
-	"github.com/neilotoole/sq/libsq/core/lg/lgm"
-
 	"github.com/neilotoole/streamcache"
 
 	"github.com/neilotoole/sq/libsq/core/errz"
 	"github.com/neilotoole/sq/libsq/core/ioz"
 	"github.com/neilotoole/sq/libsq/core/ioz/checksum"
 	"github.com/neilotoole/sq/libsq/core/ioz/httpz"
+	"github.com/neilotoole/sq/libsq/core/lg"
+	"github.com/neilotoole/sq/libsq/core/lg/lgm"
 	"github.com/neilotoole/sq/libsq/core/options"
 	"github.com/neilotoole/sq/libsq/files/downloader"
 	"github.com/neilotoole/sq/libsq/source"

testh/record.go

@@ -41,6 +41,7 @@ func RecordsFromTbl(tb testing.TB, handle, tbl string) (recMeta record.Meta, rec
 		sink, err = th.QuerySQL(src, nil, "SELECT * FROM "+tbl)
 		require.NoError(tb, err)
 		recSinkCache[key] = sink
+
 		th.Close()
 	}
 

testh/testh.go

@@ -5,7 +5,6 @@ package testh
 import (
 	"context"
 	"database/sql"
-	"io"
 	"log/slog"
 	"os"
 	"path/filepath"
@@ -105,6 +104,8 @@ type Helper struct {
 
 	initOnce sync.Once
 
+	closeOnce sync.Once
+
 	mu sync.Mutex
 }
 
@@ -210,6 +211,8 @@ func (h *Helper) init() {
 			OptionsRegistry: optRegistry,
 			DriverRegistry:  h.registry,
 		}
+
+		h.Cleanup.AddC(h.run)
 	})
 }
 
@@ -217,10 +220,12 @@ func (h *Helper) init() {
 // error occurs. Close is automatically invoked via t.Cleanup, it does
 // not need to be explicitly invoked unless desired.
 func (h *Helper) Close() {
-	err := h.Cleanup.Run()
-	lg.WarnIfError(h.Log(), "helper cleanup", err)
-	assert.NoError(h.T, err)
-	h.cancelFn()
+	h.closeOnce.Do(func() {
+		err := h.Cleanup.Run()
+		lg.WarnIfError(h.Log(), "helper cleanup", err)
+		assert.NoError(h.T, err)
+		h.cancelFn()
+	})
 }
 
 // Add adds src to the helper's collection.
@@ -312,27 +317,12 @@ func (h *Helper) Source(handle string) *source.Source {
 
 	if src.Type == drivertype.SQLite {
 		// This could be easily generalized for CSV/XLSX etc.
-		fpath, err := sqlite3.PathFromLocation(src)
+		srcPath, err := sqlite3.PathFromLocation(src)
 		require.NoError(t, err)
 
-		srcFile, err := os.Open(fpath)
-		require.NoError(t, err)
-		defer func() {
-			assert.NoError(t, srcFile.Close())
-		}()
-
-		destFile, err := h.files.CreateTemp("*_"+filepath.Base(src.Location), true)
-		require.NoError(t, err)
-		defer func() {
-			assert.NoError(t, destFile.Close())
-		}()
-
-		destFileName := destFile.Name()
-
-		_, err = io.Copy(destFile, srcFile)
-		require.NoError(t, err)
-
-		src.Location = sqlite3.Prefix + destFileName
+		dstPath := filepath.Join(tu.TempDir(t, false), filepath.Base(srcPath))
+		require.NoError(t, ioz.CopyFile(dstPath, srcPath, true))
+		src.Location = sqlite3.Prefix + dstPath
 	}
 
 	h.srcCache[handle] = src