Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
HouzuoGuo opened this issue Jan 13, 2020 · 2 comments
Open

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

HouzuoGuo opened this issue Jan 13, 2020 · 2 comments
Labels
Documentation help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@HouzuoGuo
Copy link

HouzuoGuo commented Jan 13, 2020

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.

@ianlancetaylor ianlancetaylor changed the title Please clarity the purpose of Process.Release function os: please clarify the purpose of Process.Release method Jan 13, 2020
@ianlancetaylor ianlancetaylor added Documentation help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jan 13, 2020
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Jan 13, 2020
@perillo
Copy link
Contributor

perillo commented Jan 24, 2020

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
Copy link

crazymi commented Jul 31, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants