From 67109b1731a24f107e0fac0c487cb34fbbdde427 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jes=C3=BAs=20Garc=C3=ADa=20Crespo?= <jesus@sevein.com>
Date: Wed, 4 Sep 2024 08:34:18 +0000
Subject: [PATCH] Avoid panics on exit

Update pyRunner to ensure that the process is forcibly killed if it does not
respond to the quit request. The standard streams are closed by Cmd.Wait so we
don't have to.
---
 bagit_test.go | 10 ++++++++++
 go.mod        |  4 ++--
 go.sum        |  8 ++++----
 runner.go     | 41 +++++++++++++++++++++++++++--------------
 4 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/bagit_test.go b/bagit_test.go
index 105c151..cb8a89f 100644
--- a/bagit_test.go
+++ b/bagit_test.go
@@ -139,3 +139,13 @@ Tag-File-Character-Encoding: UTF-8
 		assert.ErrorContains(t, err, "does not exist")
 	})
 }
+
+func TestCleanup(t *testing.T) {
+	t.Parallel()
+
+	b, err := bagit.NewBagIt()
+	assert.NilError(t, err)
+
+	err = b.Cleanup()
+	assert.NilError(t, err)
+}
diff --git a/go.mod b/go.mod
index b4c2c88..484b40f 100644
--- a/go.mod
+++ b/go.mod
@@ -3,8 +3,8 @@ module github.com/artefactual-labs/bagit-gython
 go 1.21
 
 require (
-	github.com/kluctl/go-embed-python v0.0.0-3.12.3-20240415-1
-	golang.org/x/sync v0.7.0
+	github.com/kluctl/go-embed-python v0.0.0-3.12.3-20240415-2
+	golang.org/x/sync v0.8.0
 	gotest.tools/v3 v3.5.1
 )
 
diff --git a/go.sum b/go.sum
index c18af82..3df32e2 100644
--- a/go.sum
+++ b/go.sum
@@ -5,8 +5,8 @@ github.com/gobwas/glob v0.2.3 h1:A4xDbljILXROh+kObIiy5kIaPYD8e96x1tgBhUI5J+Y=
 github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJAkT8=
 github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
 github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
-github.com/kluctl/go-embed-python v0.0.0-3.12.3-20240415-1 h1:IX2O3LJUL0AjYsROGZ4aNENTEb3c/Ll/0b9Yb/8P61Q=
-github.com/kluctl/go-embed-python v0.0.0-3.12.3-20240415-1/go.mod h1:9kqX8IjRCNh4ppXxlKGtLN+QFuvsdSsNGKsTLgdSNRw=
+github.com/kluctl/go-embed-python v0.0.0-3.12.3-20240415-2 h1:JcYhVgX7jFN8QcoBxx8/kLxUyeUzE/JnGf5ntulNPPM=
+github.com/kluctl/go-embed-python v0.0.0-3.12.3-20240415-2/go.mod h1:9kqX8IjRCNh4ppXxlKGtLN+QFuvsdSsNGKsTLgdSNRw=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
@@ -17,8 +17,8 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
 github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
-golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M=
-golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ=
+golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws=
 golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
diff --git a/runner.go b/runner.go
index d08f425..682d204 100644
--- a/runner.go
+++ b/runner.go
@@ -10,6 +10,7 @@ import (
 	"os/exec"
 	"sync"
 	"sync/atomic"
+	"time"
 
 	"github.com/kluctl/go-embed-python/python"
 )
@@ -135,31 +136,43 @@ func (r *pyRunner) quit() error {
 	r.mu.Lock()
 	defer r.mu.Unlock()
 
-	_, err := r.stdin.Write([]byte(`{"name": "exit"}`))
+	var err error
+	if r.stdin != nil {
+		_, err = r.stdin.Write([]byte(`{"name": "exit"}`))
+	}
 
 	return err
 }
 
-func (i *pyRunner) stop() error {
+func (r *pyRunner) stop() error {
 	var e error
 
-	if err := i.quit(); err != nil {
-		e = errors.Join(e, err)
-	}
-
-	if err := i.stdin.Close(); err != nil {
-		e = errors.Join(e, err)
-	}
-
-	if err := i.stdout.Close(); err != nil {
+	if err := r.quit(); err != nil {
 		e = errors.Join(e, err)
 	}
 
-	if err := i.cmd.Process.Kill(); err != nil {
-		e = errors.Join(e, err)
+	// Wait up to a second, otherwise force to exit immediately.
+	if closed := wait(&r.wg, time.Second); !closed {
+		if err := r.cmd.Process.Kill(); err != nil {
+			e = errors.Join(e, err)
+		}
 	}
 
-	i.wg.Wait()
+	r.wg.Wait()
 
 	return e
 }
+
+func wait(wg *sync.WaitGroup, timeout time.Duration) bool {
+	done := make(chan struct{})
+	go func() {
+		wg.Wait()
+		close(done)
+	}()
+	select {
+	case <-done:
+		return true
+	case <-time.After(timeout):
+		return false
+	}
+}