Stop the shim silently going deaf mid-command

cmd/undo/run.go

@@ -6,6 +6,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"syscall"
 
 	"github.com/edaywalid/undo/internal/session"
 )
@@ -105,6 +106,11 @@ func cmdRun(argv []string) {
 	if runErr != nil {
 		if ee, ok := runErr.(*exec.ExitError); ok {
 			code = ee.ExitCode()
+			// killed by a signal: ExitCode is -1, which os.Exit turns
+			// into 255. Report it the way a shell does.
+			if ws, ok := ee.Sys().(syscall.WaitStatus); ok && ws.Signaled() {
+				code = 128 + int(ws.Signal())
+			}
 		} else {
 			s.Remove()
 			fatal(runErr)

shim/undo_shim.c

@@ -340,37 +340,58 @@ static int ignored(const char *abs)
  * otherwise save one backup per write. Only the first (pre-command)
  * backup is needed to restore, so we record which paths have been saved
  * and skip the rest. Best-effort: once the table fills we stop deduping,
- * which only costs extra backups, never a missed one. */
+ * which only costs extra backups, never a missed one.
+ *
+ * Slots hold path hashes, not the paths, so the table owns no memory:
+ * malloc inside an interposed open() is a hazard of its own, and with
+ * nothing to free there is nothing a second thread can pull out from
+ * under us when the session changes.
+ *
+ * What that costs: two paths sharing a 64-bit hash dedup as one and the
+ * second goes unsaved. Around 1e-10 for a command touching 100k distinct
+ * paths, against allocating inside open() on every write. Worth it, but
+ * it is the one way this table can lose a backup rather than add one. */
 #define DEDUP_CAP 16384
-static char *dedup_tab[DEDUP_CAP];
+static uint64_t dedup_tab[DEDUP_CAP];
 static int dedup_count;
+static char dedup_dir[PATH_MAX];
 
-static unsigned long path_hash(const char *s)
+static uint64_t path_hash(const char *s)
 {
-    unsigned long h = 1469598103934665603UL;
+    uint64_t h = 1469598103934665603ULL;
     for (; *s; s++) {
         h ^= (unsigned char)*s;
-        h *= 1099511628211UL;
+        h *= 1099511628211ULL;
     }
-    return h;
+    return h ? h : 1; /* 0 marks an empty slot */
 }
 
 /* returns 1 if `abs` was already saved this command (skip it); otherwise
- * records it and returns 0. */
+ * records it and returns 0.
+ *
+ * The table covers one command. A shell preloaded with the shim
+ * (UNDO_CAPTURE_SHELL=1) outlives every session it runs, so a path saved
+ * for an earlier command must not suppress its backup in the next one. */
 static int mod_seen(const char *abs)
 {
+    const char *dir = session_dir();
+    if (!dir)
+        return 0;
+    if (strcmp(dedup_dir, dir) != 0) {
+        memset(dedup_tab, 0, sizeof dedup_tab);
+        dedup_count = 0;
+        snprintf(dedup_dir, sizeof dedup_dir, "%s", dir);
+    }
     if (dedup_count * 4 >= DEDUP_CAP * 3)
         return 0; /* table nearly full: stop deduping, keep saving */
-    unsigned long i = path_hash(abs) & (DEDUP_CAP - 1);
+    uint64_t h = path_hash(abs);
+    unsigned long i = h & (DEDUP_CAP - 1);
     while (dedup_tab[i]) {
-        if (strcmp(dedup_tab[i], abs) == 0)
+        if (dedup_tab[i] == h)
             return 1;
         i = (i + 1) & (DEDUP_CAP - 1);
     }
-    char *dup = strdup(abs);
-    if (!dup)
-        return 0; /* out of memory: save anyway */
-    dedup_tab[i] = dup;
+    dedup_tab[i] = h;
     dedup_count++;
     return 0;
 }
@@ -547,11 +568,9 @@ int rmdir(const char *path)
     int ok;
     handle_rmdir_pre(AT_FDCWD, path, abs, mode, &ok);
     int rc = real_rmdir(path);
-    if (rc == 0 && ok) {
-        in_shim = 1;
+    if (rc == 0 && ok)
         jwrite("rmdir", abs, mode, NULL);
-        in_shim = 0;
-    }
+    in_shim = 0;
     return rc;
 }
 

test/e2e.sh

@@ -204,7 +204,47 @@ else
     echo "   (no cc, skipped)"
 fi
 
-echo "== case 20: undo doctor passes its live self-test"
+echo "== case 20: a failed rmdir does not stop the rest of the command being recorded"
+mkdir -p "$PLAY/full/x" "$PLAY/gone"
+run_armed "rmdir $PLAY/full $PLAY/gone || true"
+[[ -d $PLAY/full && ! -d $PLAY/gone ]] || fail "rmdir did not run as expected"
+"$UNDO" -y >/dev/null
+[[ -d $PLAY/gone ]] || fail "second rmdir was not journaled after the first failed"
+
+# One long-lived process writing the same file under two sessions in a
+# row: what UNDO_CAPTURE_SHELL=1 does, where the shim is loaded into the
+# shell itself rather than into a fresh child per command.
+if command -v python3 >/dev/null 2>&1; then
+    echo "== case 21: one process spanning two sessions backs up in both"
+    f=$PLAY/shared.txt
+    echo v0 >"$f"
+    s1=$UNDO_DATA_DIR/sessions/$(date +%s%N | cut -c1-16); mkdir -p "$s1/data"
+    echo "write v1" >"$s1/cmd"; sleep 0.01
+    s2=$UNDO_DATA_DIR/sessions/$(date +%s%N | cut -c1-16); mkdir -p "$s2/data"
+    echo "write v2" >"$s2/cmd"
+    LD_PRELOAD="$LIB" python3 -c "
+import os, sys
+for sess, body in ((sys.argv[1], 'v1'), (sys.argv[2], 'v2')):
+    os.environ['UNDO_SESSION'] = sess
+    with open(sys.argv[3], 'w') as fh:
+        fh.write(body + '\n')
+" "$s1" "$s2" "$f"
+    [[ $(cat "$f") == v2 ]] || fail "writes did not run"
+    grep -q "shared.txt" "$s2/journal" 2>/dev/null || fail "second session recorded nothing"
+    "$UNDO" -y >/dev/null
+    [[ $(cat "$f") == v1 ]] || fail "second session did not restore v1, got $(cat "$f")"
+else
+    echo "== case 21: skipped (no python3)"
+fi
+
+echo "== case 22: undo run reports a signal death the way a shell does"
+set +e
+"$UNDO" run -- sh -c 'kill -TERM $$' >/dev/null 2>&1
+rc=$?
+set -e
+[[ $rc == 143 ]] || fail "expected 143 from a SIGTERM death, got $rc"
+
+echo "== case 23: undo doctor passes its live self-test"
 out=$("$UNDO" doctor 2>&1) || fail "doctor exited non-zero: $out"
 grep -q "\[ok  \] capture" <<<"$out" || fail "doctor capture check did not pass"
 grep -q "\[ok  \] restore" <<<"$out" || fail "doctor restore check did not pass"