amadan
stopevidence: a kcmp ESRCH skips a thread only once its task directory is gone, and is an incomplete scan while the task is still present
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+69
−5
internal/stopevidence/writers_linux.go+15 −5internal/stopevidence/writers_linux_test.go+54 −0
diff --git a/internal/stopevidence/writers_linux.go b/internal/stopevidence/writers_linux.go| index c59068794..dbaf5167a 100644 |
| --- a/internal/stopevidence/writers_linux.go |
| +++ b/internal/stopevidence/writers_linux.go |
| @@ -127,8 +127,9 @@ func scanProcess(ctx context.Context, root string, pid int, paths []string) ([]D |
| // database at the same number. Any other thread's descriptor is reported |
| // with FD -1, so no allow-list can exempt it. A comparison that cannot be |
| // made (ENOSYS, EPERM, anything but a vanished task) is ErrIncomplete. A |
| -// process or thread that vanished mid-scan is skipped; an unreadable |
| -// table is ErrIncomplete. |
| +// process or thread that vanished mid-scan is skipped, and a kcmp ESRCH |
| +// counts as vanished only once the task directory is gone; an unreadable |
| +// table, or an ESRCH for a task still present, is ErrIncomplete. |
| func scanOne(ctx context.Context, root string, pid int, files fileSet, ops fdOps, cmp kcmpFunc) ([]Descriptor, error) { |
| pidDir := filepath.Join(root, strconv.Itoa(pid)) |
| tasks, err := readProcDir(filepath.Join(pidDir, "task"), pidDir, ops) |
| @@ -189,7 +190,16 @@ func scanOne(ctx context.Context, root string, pid int, files fileSet, ops fdOps |
| return nil, fmt.Errorf("%w: pid %d tid %d fd %d: kcmp: %w", ErrIncomplete, pid, tid, n, err) |
| } |
| if gone { |
| - break descriptors // the thread exited mid-scan |
| + // kcmp's ESRCH alone is no evidence the thread |
| + // exited: only its task directory being gone is. |
| + _, lerr := ops.lstat(taskDir) |
| + if procGone(lerr) { |
| + break descriptors // the thread exited mid-scan |
| + } |
| + if lerr == nil { |
| + lerr = errors.New("the task is still present") |
| + } |
| + return nil, fmt.Errorf("%w: pid %d tid %d fd %d: kcmp: %w: %w", ErrIncomplete, pid, tid, n, unix.ESRCH, lerr) |
| } |
| if !ok { |
| d.FD = -1 // not provably the leader's descriptor |
| @@ -207,8 +217,8 @@ func scanOne(ctx context.Context, root string, pid int, files fileSet, ops fdOps |
| // leadersDescriptor is the kernel's evidence that thread tid's descriptor |
| // n is the leader's own: the same table (KCMP_FILES, asked once per thread |
| // and kept in *shared) and the same open file description (KCMP_FILE). |
| -// sameName is the leader showing n on the same path. gone reports a task |
| -// that exited (ESRCH); EBADF (n closed in one of them since its listing) |
| +// sameName is the leader showing n on the same path. gone reports kcmp's |
| +// ESRCH, which the caller confirms against the task directory; EBADF (n closed in one of them since its listing) |
| // is "not the leader's"; any other error is returned. |
| func leadersDescriptor(cmp kcmpFunc, pid, tid, n int, sameName bool, shared *int) (ok, gone bool, err error) { |
| if !sameName { |
diff --git a/internal/stopevidence/writers_linux_test.go b/internal/stopevidence/writers_linux_test.go| index eb242af4b..00312ce59 100644 |
| --- a/internal/stopevidence/writers_linux_test.go |
| +++ b/internal/stopevidence/writers_linux_test.go |
| @@ -237,6 +237,60 @@ func TestWritersStoppedRefusesAnyOtherHolder(t *testing.T) { |
| } |
| }) |
| } |
| + // Final review Important 1: kcmp's ESRCH is a vanished thread only once |
| + // its task directory is gone. A thread still listed keeps its holders. |
| + for _, typ := range []int{kcmpFiles, kcmpFile} { |
| + t.Run("kcmp ESRCH for a thread still present: type "+strconv.Itoa(typ), func(t *testing.T) { |
| + root, db := writersProc(t) |
| + prior := kcmpFor |
| + kcmpFor = func(string) kcmpFunc { |
| + return func(pid, tid, got int, a, b uintptr) (int, error) { |
| + if got == typ { |
| + return 0, unix.ESRCH |
| + } |
| + return 0, nil |
| + } |
| + } |
| + defer func() { kcmpFor = prior }() |
| + if _, err := WritersStopped(t.Context(), writersInputs(root, db)); !errors.Is(err, ErrIncomplete) { |
| + t.Fatalf("WritersStopped: %v", err) |
| + } |
| + if _, err := ProcessDescriptors(t.Context(), root, 20, []string{db}); !errors.Is(err, ErrIncomplete) { |
| + t.Fatalf("own descriptors: %v", err) |
| + } |
| + }) |
| + t.Run("kcmp ESRCH for a thread that exited: type "+strconv.Itoa(typ), func(t *testing.T) { |
| + root, db := writersProc(t) |
| + // Thread 8 holds the database on a descriptor the leader does |
| + // not show, then exits: it is skipped, not a holder. |
| + dir := filepath.Join(root, "20", "task", "8", "fd") |
| + if err := os.MkdirAll(dir, 0700); err != nil { |
| + t.Fatal(err) |
| + } |
| + for fd, target := range map[int]string{5: db, 6: db + "-wal"} { |
| + if err := os.Symlink(target, filepath.Join(dir, strconv.Itoa(fd))); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + prior := kcmpFor |
| + kcmpFor = func(r string) kcmpFunc { |
| + real := fixtureKcmp(r) |
| + return func(pid, tid, got int, a, b uintptr) (int, error) { |
| + if tid == 8 && got == typ { |
| + if err := os.RemoveAll(filepath.Join(root, "20", "task", "8")); err != nil { |
| + t.Error(err) |
| + } |
| + return 0, unix.ESRCH |
| + } |
| + return real(pid, tid, got, a, b) |
| + } |
| + } |
| + defer func() { kcmpFor = prior }() |
| + if _, err := WritersStopped(t.Context(), writersInputs(root, db)); err != nil { |
| + t.Fatalf("WritersStopped: %v", err) |
| + } |
| + }) |
| + } |
| t.Run("an unreadable descriptor directory", func(t *testing.T) { |
| root, db := writersProc(t) |
| if _, err := WritersStopped(t.Context(), writersInputs(filepath.Join(root, "missing"), db)); !errors.Is(err, ErrIncomplete) { |