Wait quietly for clipmenud's session lock; count the operator's clipmenud (issues 860, 862)

modules/clipmenu/cmd/clipmenu-tools/clipboard_test.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"os"
 	"os/exec"
+	"os/user"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -225,3 +226,27 @@ func TestWithoutAStoreTheChangesSayWhy(t *testing.T) {
 		t.Fatalf("an empty history: %+v, %v", h, err)
 	}
 }
+
+// The tools can run as another account than the operator's; the daemon that counts is the operator's
+// (issue 860). Here the operator is the test's own account and the tool runs as a different uid.
+func TestCollectingIsTheOperatorsDaemonNotThatOfTheAccountTheToolRunsAs(t *testing.T) {
+	store(t, map[string]string{"first": "first"}, "first")
+	fakeProcess(t, nobody, "clipmenud") // owned by the test's uid: the operator's
+	u, err := user.Current()
+	if err != nil {
+		t.Skip(err)
+	}
+	t.Setenv("MESH_OPERATOR_ACCOUNT", u.Username)
+	old := toolUID
+	toolUID = func() int { return os.Getuid() + 1 }
+	t.Cleanup(func() { toolUID = old })
+	h, err := History(10)
+	if err != nil || !h.Collecting {
+		t.Fatalf("the operator's clipmenud runs: %+v %v", h, err)
+	}
+	// without the operator named, the tool's own uid stands, and that account has no daemon
+	t.Setenv("MESH_OPERATOR_ACCOUNT", "")
+	if h, _ := History(10); h.Collecting {
+		t.Fatalf("another account's daemon counted: %+v", h)
+	}
+}

modules/clipmenu/cmd/clipmenu-tools/manifest_test.go

@@ -7,6 +7,8 @@ import (
 	"os/exec"
 	"reflect"
 	"strings"
+	"sync"
+	"syscall"
 	"testing"
 	"time"
 )
@@ -145,3 +147,78 @@ func TestTheLauncherUsesTheManagersDisplayAndWaitsOtherwise(t *testing.T) {
 		t.Fatalf("the display that arrived while waiting: %q %v", out, ok)
 	}
 }
+
+// Run for real: while another clipmenud of the account holds the session lock the launcher waits without
+// a word, and starts clipmenud once the lock is free (issue 862).
+func TestTheLauncherWaitsQuietlyWhileAnotherClipmenudHoldsTheLock(t *testing.T) {
+	if _, err := exec.LookPath("flock"); err != nil {
+		t.Skip("flock is missing here")
+	}
+	m := readManifest(t)
+	script, _ := m.resource(t, "launcher")["content"].(string)
+	dir := t.TempDir()
+	body := strings.Replace(script, "exec clipmenud", `echo "started"`, 1)
+	file := dir + "/launcher"
+	if err := os.WriteFile(file, []byte(body), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	store := dir + "/cm/clipmenu.6.op"
+	os.MkdirAll(store, 0o700)
+	lock := store + "/session_lock"
+	// the older clipmenud: it holds the lock the way clipmenud does, until it is killed
+	holder := exec.Command("flock", "-x", "-n", lock, "sleep", "60")
+	holder.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	if err := holder.Start(); err != nil {
+		t.Fatal(err)
+	}
+	// flock's child shares the lock, so the whole group goes
+	release := func() { syscall.Kill(-holder.Process.Pid, syscall.SIGKILL); holder.Wait() }
+	defer release()
+	for i := 0; i < 50; i++ {
+		if probe := exec.Command("flock", "-n", lock, "true"); probe.Run() != nil {
+			break
+		}
+		time.Sleep(50 * time.Millisecond)
+	}
+	var out syncBuffer
+	cmd := exec.Command("sh", file)
+	cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "HOME=" + dir, "DISPLAY=:7", "USER=op", "CM_DIR=" + dir + "/cm"}
+	cmd.Stdout, cmd.Stderr = &out, &out
+	if err := cmd.Start(); err != nil {
+		t.Fatal(err)
+	}
+	done := make(chan error, 1)
+	go func() { done <- cmd.Wait() }()
+	select {
+	case <-done:
+		t.Fatalf("the launcher went on while the lock was held: %q", out.String())
+	case <-time.After(2500 * time.Millisecond):
+	}
+	if out.Len() != 0 {
+		t.Fatalf("the launcher spoke while it waited: %q", out.String())
+	}
+	release()
+	select {
+	case <-done:
+	case <-time.After(5 * time.Second):
+		cmd.Process.Kill()
+		t.Fatal("the launcher did not start clipmenud after the lock was freed")
+	}
+	if got := strings.TrimSpace(out.String()); got != "started" {
+		t.Fatalf("after the lock was freed: %q", got)
+	}
+}
+
+type syncBuffer struct {
+	mu sync.Mutex
+	b  strings.Builder
+}
+
+func (s *syncBuffer) Write(p []byte) (int, error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.b.Write(p)
+}
+
+func (s *syncBuffer) Len() int       { s.mu.Lock(); defer s.mu.Unlock(); return s.b.Len() }
+func (s *syncBuffer) String() string { s.mu.Lock(); defer s.mu.Unlock(); return s.b.String() }

modules/clipmenu/cmd/clipmenu-tools/session.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"os/user"
 	"path/filepath"
 	"sort"
 	"strconv"
@@ -386,13 +387,31 @@ func (c *capped) Write(p []byte) (int, error) {
 	return c.Buffer.Write(p)
 }
 
-// processesOf are the pids of this uid's processes whose command name is comm, oldest first.
+// toolUID is the uid the tool runs as; a test replaces it to stand for a tool that runs as another
+// account than the operator's.
+var toolUID = os.Getuid
+
+// operatorUID is the uid of the operator's account, whose clipmenud and store these tools look at.
+// The runtime serves the tools as an account that need not be the operator's and names the operator
+// in MESH_OPERATOR_ACCOUNT; without it (or when it is unknown here) the tool's own uid stands.
+func operatorUID() int {
+	if name := strings.TrimSpace(os.Getenv("MESH_OPERATOR_ACCOUNT")); name != "" {
+		if u, err := user.Lookup(name); err == nil {
+			if id, err := strconv.Atoi(u.Uid); err == nil {
+				return id
+			}
+		}
+	}
+	return toolUID()
+}
+
+// processesOf are the pids of the operator account's processes whose command name is comm, oldest first.
 func processesOf(comm string) []int {
 	entries, err := os.ReadDir(procRoot)
 	if err != nil {
 		return nil
 	}
-	uid := os.Getuid()
+	uid := operatorUID()
 	var pids []int
 	for _, e := range entries {
 		pid, err := strconv.Atoi(e.Name())
@@ -411,7 +430,7 @@ func processesOf(comm string) []int {
 	return pids
 }
 
-// signalAll sends sig to every process of this uid named comm, and answers the pids it reached.
+// signalAll sends sig to every process of the operator account named comm, and answers the pids it reached.
 func signalAll(comm string, sig syscall.Signal) []int {
 	var reached []int
 	for _, pid := range processesOf(comm) {