Apologies for reviving an old thread, but thought this might be the most
efficient way to keep context.
I'm looking at a similar issue Thomas encountered, but in this case I'm
concerned about how to handle errors returned from
StdoutPipe()/StderrPipe() and properly clean up after that. e.g.:
cmd := exec.Command(myCommand, myArgs...)
stdout, err := cmd.StdoutPipe()
if err != nil {
// log warning or bail out
}
stderr, err := cmd.StderrPipe()
if err != nil {
// log warning, if we bail out we will leak the os.Pipe allocated
internally in cmd.StdoutPipe()
}
err = cmd.Start() // if this fails, it *will* clean up both pipes
Looking at the source, both StdoutPipe() and StderrPipe() allocate
os.Pipe()s internally. Since these methods return the reader I can
technically close that one myself but the writer side of the pipe will
presumably leak if cmd.Start() is never called. In my usecase it's probably
an acceptable compromise to just complain to the log and move along with
starting the process, but if my understanding is correct it feels like an
omission in the language spec that there is seemingly no way to clean up a
the pipes associated with a command that is never started.
Thanks,
Lucas
On Saturday, December 14, 2019 at 8:05:00 PM UTC-8 Ian Lance Taylor wrote:
> On Sat, Dec 14, 2019 at 7:40 PM <tomas...@showmax.com> wrote:
> >
> > I need to spawn a subprocess and I'm trying to use os/exec. I need to
> capture
> > stdout/err so I'm using Cmd.StdoutPipe and Cmd.StderrPipe . The problem
> I'm
> > having is how to correctly clean up the pipes after I am done. If
> Cmd.Start
> > works fine, all I need to do is read everything and then call Cmd.Wait,
> which
> > states that it will clean up ( https://golang.org/pkg/os/exec/#Cmd.Wait
> ):
> >
> > > Wait releases any resources associated with the Cmd.
> >
> > However, what I am not sure about is how to handle cases when Cmd.Start
> returns
> > an error. Nowhere in its documentation it's said that it cleans up the
> > resources, so originally I thought I have to do the cleanup myself.
> However,
> > after checking the source code it does seem to close the pipes when it
> fails:
> >
> > https://golang.org/src/os/exec/exec.go?s=11462:11489#L374
> >
> > And now I'm not sure what to do. io.Closer states that double-close is
> no-no:
> >
> > > The behavior of Close after the first call is undefined.
> >
> > so I cannot even close them myself (since they will be already closed).
> It seems
> > I have to rely on undocumented behaviour, but those have tendency to
> change and
> > I don't want to start leaking file descriptors after patch version
> update... I
> > assume there must be a way to solve this using just documented
> behaviour. Can
> > someone point me in a right direction?
> >
> >
> >
> > Maybe to better show my problem, here it's in pseudo code (error handling
> > omitted):
> >
> > cmd := exec.Command(name, args...)
> > stdout, _ := cmd.StdoutPipe()
> > stderr, _ := cmd.StderrPipe()
> > if err := cmd.Start(); err != nil {
> > // --> How should I close stdout/stderr here to future-proof for
> > // changes in cmd.Start behavior?
> > return err
> > }
> > consumeAll(stdout)
> > consumeAll(stderr)
> > cmd.Wait()
> > return nil
>
>
> If Start returns an error, the read and write ends of the pipes will
> be closed just as though Wait returned. You don't have to close
> anything yourself if Start fails. I agree that this should be
> documented.
>
> Ian
>
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/golang-nuts/5a639c04-f75d-4da1-86da-54d4fa4bfef3n%40googlegroups.com.
