Make threadsafe access to Processes and remove own unused Kill

Author: zeripath

modules/process/manager.go

@@ -12,6 +12,7 @@ import (
 	"fmt"
 	"io"
 	"os/exec"
+	"sort"
 	"sync"
 	"time"
 )
@@ -43,14 +44,14 @@ type Manager struct {
 	mutex sync.Mutex
 
 	counter   int64
-	Processes map[int64]*Process
+	processes map[int64]*Process
 }
 
 // GetManager returns a Manager and initializes one as singleton if there's none yet
 func GetManager() *Manager {
 	if manager == nil {
 		manager = &Manager{
-			Processes: make(map[int64]*Process),
+			processes: make(map[int64]*Process),
 		}
 	}
 	return manager
@@ -60,7 +61,7 @@ func GetManager() *Manager {
 func (pm *Manager) Add(description string, cmd *exec.Cmd, cancel context.CancelFunc) int64 {
 	pm.mutex.Lock()
 	pid := pm.counter + 1
-	pm.Processes[pid] = &Process{
+	pm.processes[pid] = &Process{
 		PID:         pid,
 		Description: description,
 		Start:       time.Now(),
@@ -76,20 +77,32 @@ func (pm *Manager) Add(description string, cmd *exec.Cmd, cancel context.CancelF
 // Remove a process from the ProcessManager.
 func (pm *Manager) Remove(pid int64) {
 	pm.mutex.Lock()
-	delete(pm.Processes, pid)
+	delete(pm.processes, pid)
 	pm.mutex.Unlock()
 }
 
 // Cancel a process in the ProcessManager.
 func (pm *Manager) Cancel(pid int64) {
 	pm.mutex.Lock()
-	process, ok := pm.Processes[pid]
+	process, ok := pm.processes[pid]
 	pm.mutex.Unlock()
 	if ok {
 		process.Cancel()
 	}
 }
 
+// Processes gets the processes in a thread safe manner
+func (pm *Manager) Processes() []*Process {
+	pm.mutex.Lock()
+	processes := make([]*Process, 0, len(pm.processes))
+	for _, process := range pm.processes {
+		processes = append(processes, process)
+	}
+	pm.mutex.Unlock()
+	sort.Sort(processList(processes))
+	return processes
+}
+
 // Exec a command and use the default timeout.
 func (pm *Manager) Exec(desc, cmdName string, args ...string) (string, string, error) {
 	return pm.ExecDir(-1, "", desc, cmdName, args...)
@@ -152,24 +165,16 @@ func (pm *Manager) ExecDirEnvStdIn(timeout time.Duration, dir, desc string, env
 	return stdOut.String(), stdErr.String(), err
 }
 
-// Kill and remove a process from list.
-func (pm *Manager) Kill(pid int64) error {
-	if proc, exists := pm.Processes[pid]; exists {
-		pm.mutex.Lock()
-		if proc.Cancel != nil {
-			proc.Cancel()
-		}
-		if proc.Cmd != nil &&
-			proc.Cmd.Process != nil &&
-			proc.Cmd.ProcessState != nil &&
-			!proc.Cmd.ProcessState.Exited() {
-			if err := proc.Cmd.Process.Kill(); err != nil {
-				return fmt.Errorf("failed to kill process(%d/%s): %v", pid, proc.Description, err)
-			}
-		}
-		delete(pm.Processes, pid)
-		pm.mutex.Unlock()
-	}
+type processList []*Process
+
+func (l processList) Len() int {
+	return len(l)
+}
+
+func (l processList) Less(i, j int) bool {
+	return l[i].PID < l[j].PID
+}
 
-	return nil
+func (l processList) Swap(i, j int) {
+	l[i], l[j] = l[j], l[i]
 }

modules/process/manager_test.go

@@ -10,7 +10,7 @@ import (
 )
 
 func TestManager_Add(t *testing.T) {
-	pm := Manager{Processes: make(map[int64]*Process)}
+	pm := Manager{processes: make(map[int64]*Process)}
 
 	pid := pm.Add("foo", exec.Command("foo"), nil)
 	assert.Equal(t, int64(1), pid, "expected to get pid 1 got %d", pid)
@@ -20,7 +20,7 @@ func TestManager_Add(t *testing.T) {
 }
 
 func TestManager_Cancel(t *testing.T) {
-	pm := Manager{Processes: make(map[int64]*Process)}
+	pm := Manager{processes: make(map[int64]*Process)}
 
 	ctx, cancel := context.WithCancel(context.Background())
 	pid := pm.Add("foo", exec.Command("foo"), cancel)
@@ -35,7 +35,7 @@ func TestManager_Cancel(t *testing.T) {
 }
 
 func TestManager_Remove(t *testing.T) {
-	pm := Manager{Processes: make(map[int64]*Process)}
+	pm := Manager{processes: make(map[int64]*Process)}
 
 	pid1 := pm.Add("foo", exec.Command("foo"), nil)
 	assert.Equal(t, int64(1), pid1, "expected to get pid 1 got %d", pid1)
@@ -45,7 +45,7 @@ func TestManager_Remove(t *testing.T) {
 
 	pm.Remove(pid2)
 
-	_, exists := pm.Processes[pid2]
+	_, exists := pm.processes[pid2]
 	assert.False(t, exists, "PID %d is in the list but shouldn't", pid2)
 }
 

routers/admin/admin.go

@@ -352,7 +352,7 @@ func Monitor(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("admin.monitor")
 	ctx.Data["PageIsAdmin"] = true
 	ctx.Data["PageIsAdminMonitor"] = true
-	ctx.Data["Processes"] = process.GetManager().Processes
+	ctx.Data["Processes"] = process.GetManager().Processes()
 	ctx.Data["Entries"] = cron.ListTasks()
 	ctx.HTML(200, tplMonitor)
 }