Similar Issues

• os: TestKillFindProcess failures with "Test process succeeded, but expected to fail" #44131
• os: timeout on NetBSD with os.(*Process).Wait #56215
• os/exec: TestIgnorePipeErrorOnSuccess/10MB often hangs on netbsd-amd64-71 #20835
• os: on NetBSD, *Process.Wait sometimes deadlocks after cmd.Process.Signal returns "process already finished" #48789
• os: "panic: Fail in goroutine after TestKillStartProcess has completed" #43722

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

Oh, I missed that Wait doesn't use convertESRCH. It seems like it probably should? Though to be conservative in the freeze, just making the test accept ESRCH as well is probably best.

I think this would fail on Linux without pidfd as well. I guess we don't have any Linux builders with old enough kernels to cover non-pidfd.

@prattmic PTAL. I think that TestProcessAlreadyDone has a wrong assumption that Wait on a non-existent process should return ErrProcessDone, while in fact it does it only on Linux when pidfd is working (because statusDone is set by FindProcess).

Or, if the assumption is right and we want to tidy up the API, we should add code converting ECHILD (not ESRCH) to ErrProcessDone.

BTW there's a test very similar to TestProcessAlreadyDone but it only runs on Linux and only if pidfd is working.

I think this would fail on Linux without pidfd as well. I guess we don't have any Linux builders with old enough kernels to cover non-pidfd.

An idea, perhaps we can test non-pidfd functionality on linux, too; let me take a look.

I guess the easiest path forward is to remove the test case (as it has a wrong assumption for general case, and the specific case (when the assumption is right) is already tested by TestFindProcessViaPidfd.

Oops, I assumed that text was ESRCH, indeed it is ECHILD.

I think for this release we should change the statusDone case in Wait for pidfd to return ECHILD instead of ErrProcessDone. That makes the PID and pidfd cases return the same error (as-is, returning ErrProcessDone is a slight API change).

For next release, I do think we should consider changing all of the ECHILD cases to ErrProcessDone, as I agree it is cleaner.

think for this release we should change the statusDone case in Wait for pidfd to return ECHILD instead of ErrProcessDone.

...wrapped in NewSyscallError("wait").

Yes you're probably right. Meaning, TestFindProcessViaPidfd needs to be changed or removed as well.

(and I was hoping I just found a way to find out if pidfd is used)

...wrapped in NewSyscallError("wait").

Thanks, you saved me a round of CL review. :)

(and I was hoping I just found a way to find out if pidfd is used)

It isn't the cleanest, but given the way os is structured now, if you do the equivalent of pidfdWorks, then you know pidfd will always be used.

Change https://go.dev/cl/591816 mentions this issue: os: always return syscall.ECHILD from Wait for done process