Go: os: export errFinished as ErrProcessDone

[inyutin]

In Linux, kill() can return there possible errors:

[EINVAL]
The value of the sig argument is an invalid or unsupported signal number.

[EPERM]
The process does not have permission to send the signal to any receiving process.

[ESRCH]
No process or process group can be found corresponding to that specified by pid.

This is how os.signal function works in Go:
https://github.com/golang/go/blob/fca286bed3ed0e12336532cc711875ae5b3cb02a/src/os/exec_unix.go#L64

func (p *Process) signal(sig Signal) error {
    if p.Pid == -1 {
        return errors.New("os: process already released")
    }
    if p.Pid == 0 {
        return errors.New("os: process not initialized")
    }
    p.sigMu.RLock()
    defer p.sigMu.RUnlock()
    if p.done() {
        return errFinished
    }
    s, ok := sig.(syscall.Signal)
    if !ok {
        return errors.New("os: unsupported signal type")
    }
    if e := syscall.Kill(p.Pid, s); e != nil {
        if e == syscall.ESRCH {
            return errFinished
        }
        return e
    }
    return nil
}

What it returns private errFinished instead of public ESRCH ?
Because of that I can't write some custom logic like this:

err := process.Kill()
if err == syscall.ESRCH {
  // process already finished do something
}

It's okay that this error can have custom name, but why it private? Why errFinished, but not ErrFinished?

[ianlancetaylor]

It returns errFinished because ESRCH doesn't really seem right if the process has already exited. Also see #7658.

If you really need to distinguish these cases, which means that you are already doing Unix-specific things, I recommend that you use syscall.Kill(process.Pid, syscall.SIGKILL).

[inyutin]

It returns errFinished because ESRCH doesn't really seem right if the process has already exited. Also see #7658.

If you really need to distinguish these cases, which means that you are already doing Unix-specific things, I recommend that you use syscall.Kill(process.Pid, syscall.SIGKILL).

Thank you for answer, that's all make a point. I wish to use os.signal, because it already have lot's of useful logic like rw-locks and pid checking. If I use raw syscall.Kill(process.Pid, syscall.SIGKILL), that I need to implement the same logic on my own, in my application.

Are there any disadvantages from making errFinished public?

[ianlancetaylor]

OK, I'll turn this into a proposal to export errFinished.

[rsc]

I'm a little hesitant about exporting errFinished under that name, since it's not a general "something finished". I'm also having a hard time coming up with a better name. The wait man page says "wait for process termination" so we could use ErrProcessTerminated but that's not in keeping with the other names in os (too long, and also too specific).

@ianlancetaylor pointed out maybe we could use ErrNotExist, or perhaps we could keep using errFinished but make errors.Is(errFinished, ErrNotExist) true.

Thoughts?

[rsc]

As I noted last week, it seems like the least API surface would be to make errFinished be errors.Is(..., ErrNotExist). Retitled to that. Does anyone object to that?

[networkimprov]

Does that mean os.IsNotExist(err) == true for os.errFinished? That could be surprising, since the former is for filesystem errors. In many situations, os.IsNotExist(err) is a normal condition.

I'd expect signal and other process-related APIs to return errors specific to processes.

cc @alexbrainman @bcmills @aclements

[bcmills]

Thinking about method calls in grammatical terms, I expect os.ErrNotExist to indicate “the direct object (of the verb) does not exist”:

• os.ErrNotExist from Stat(f) indicates “could not Stat [verb] f [direct object] because f does not exist.”
• os.ErrNotExist from Open(f) indicates “could not Open [verb] f [direct object] because f does not exist.”

The proposed usage would be consistent with that, but requires the reader to treat the name of the method as the verb (as they should, but may not always do).

• “Could not Signal [verb] p [direct object] with sig [object of preposition] because p does not exist.”
• “Could not send [verb] Signal sig [direct object] to p [indirect object] because sig does not exist.”

On the other hand, the error text of os.ErrNotExist is specifically documented as file does not exist, and as @networkimprov notes, there is no file involved in Signal unless you consider the PID itself to be a file. I would not expect most users to make that association.

I think a more general standardized error for “[direct object] does not exist” could be useful, but I agree that, because of its specific text, os.ErrNotExist is not _quite_ that error.

[rsc]

@bcmills, it's not as clear cut as you say. I wouldn't be surprised if f.Stat() could return ErrNotExist in certain cases (those cases do not include "local file on Unix"). In any event, it's likely clear to the caller that the signal itself does exist.

On the other hand, ErrNotExist.String() is "file does not exist", so that's a show-stopper.

ErrProcessDone? (at least we have established Done in context, and it's shorter than Finished)

[rsc]

No responses for ErrProcessDone, but it seems like people are in general OK with exporting the error.
Given the lack of any objections, it sounds like this is a likely accept.

[rsc]

No change in consensus, so accepted.

[gopherbot]

Change https://golang.org/cl/242998 mentions this issue: os: export errFinished as ErrProcessDone

[gopherbot]

Change https://golang.org/cl/274477 mentions this issue: doc/go1.16: document os package changes