FAQ
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go


Description:
testing: fix example test fd leak

Close the read side of the pipe.
Fixes issue 4551.

Please review this at https://codereview.appspot.com/6962049/

Affected files:
M src/pkg/testing/example.go


Index: src/pkg/testing/example.go
===================================================================
--- a/src/pkg/testing/example.go
+++ b/src/pkg/testing/example.go
@@ -45,6 +45,7 @@
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
}
+ defer r.Close()
os.Stdout, os.Stderr = w, w
outC := make(chan string)
go func() {

Search Discussions

  • Russ Cox at Dec 20, 2012 at 7:19 pm
    LGTM

    Thanks.
  • Russ Cox at Dec 20, 2012 at 7:20 pm
    It looks like maybe you need to run 'hg update default' to switch to
    the main development branch? This patch doesn't apply cleanly, I think
    because it is against Go 1.0.3.
  • C Emil Hessman at Dec 20, 2012 at 8:54 pm
    Hello golang-dev@googlegroups.com, rsc@golang.org (cc:
    golang-dev@googlegroups.com),

    Please take another look.


    https://codereview.appspot.com/6962049/
  • C Emil Hessman at Dec 20, 2012 at 8:56 pm
    Hello golang-dev@googlegroups.com, rsc@golang.org (cc:
    golang-dev@googlegroups.com),

    Please take another look.


    https://codereview.appspot.com/6962049/
  • Rsc at Dec 22, 2012 at 3:19 pm
    https://codereview.appspot.com/6962049/diff/14001/src/pkg/testing/example.go
    File src/pkg/testing/example.go (right):

    https://codereview.appspot.com/6962049/diff/14001/src/pkg/testing/example.go#newcode48
    src/pkg/testing/example.go:48: defer r.Close()
    I'm sorry I didn't notice this before, but using defer here means that
    r.Close won't happen until the function returns, meaning one fd will
    leak per loop iteration. That's an improvement, since they will
    eventually get cleaned up, but it would be even better to close r once
    we're done reading it, probably right after io.Copy returns.

    https://codereview.appspot.com/6962049/
  • C Emil Hessman at Dec 22, 2012 at 4:48 pm
    Hello rsc@golang.org (cc: golang-dev@googlegroups.com),

    Please take another look.


    https://codereview.appspot.com/6962049/
  • Rsc at Dec 22, 2012 at 6:41 pm
    *** Submitted as
    https://code.google.com/p/go/source/detail?r=1e5437c42bcc ***

    testing: fix example test fd leak

    Close the read side of the pipe.
    Fixes issue 4551.

    R=rsc
    CC=golang-dev
    https://codereview.appspot.com/6962049

    Committer: Russ Cox <rsc@golang.org>


    https://codereview.appspot.com/6962049/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedDec 19, '12 at 8:51p
activeDec 22, '12 at 6:41p
posts8
users2
websitegolang.org

2 users in discussion

Rsc: 4 posts C Emil Hessman: 4 posts

People

Translate

site design / logo © 2022 Grokbase