golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.06k stars 17.68k forks source link

os: please clarify the purpose of Process.Release method #36534

Open HouzuoGuo opened 4 years ago

HouzuoGuo commented 4 years ago

What version of Go are you using (go version)?

go version go1.13.5 linux/amd64

Does this issue reproduce with the latest release?

Yes indeed.

What operating system and processor architecture are you using (go env)?

linux on amd64

What did you do?

Run the code below:

package main

import (
        "fmt"
        "os/exec"
        "time"
)

func main() {
        p := exec.Command("/usr/bin/yes")
        if err := p.Start(); err != nil {
                panic(err)
        }
        if err := p.Process.Kill(); err != nil {
                panic(err)
        }
        fmt.Println("yes is now a zombie process")
        time.Sleep(10 * time.Second)
        if err := p.Process.Release(); err != nil {
                panic(err)
        }
        fmt.Println("yes remains a zombie process")
        time.Sleep(10 * time.Second)
}

What did you expect to see?

The documentation of Process.Release states:

Release releases any resources associated with the Process p, rendering it unusable in the future. Release only needs to be called if Wait is not.

It leaves a false impression that caller should invoke Release() if the caller does not intend to consume the process exit status - "... only needs to be called if Wait is not", even though doing so leaves a zombie process on the system.

Please clarify that Release() is not intended to replace Wait() at all in the function's description.

perillo commented 4 years ago

The documentation is actually correct on Windows. Maybe on UNIX systems Release should send the pid to a reaper, before setting Process.Pid to -1.

The alternative is to document that Release behavior is different between UNIX systems and Windows.

crazymi commented 4 years ago

@HouzuoGuo , @perillo I've sent the PR that specifying Release's NOOP behavior on Unix. Feel free to recommend me better docstring. :)