code review 5490075: exec: add test to verify net package's epoll fd doesn't...

exec: add test to verify net package's epoll fd doesn't go to child

[bradfitz, 2011-12-19 16:53:46]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2011-12-19 16:54:59]

I would like this tested on all the Unixes, not just Linux.
What about the 'open lots of FDs' idea?

[bradfitz, 2011-12-19 16:56:22]

Will do.

On Mon, Dec 19, 2011 at 8:54 AM, Russ Cox <rsc@golang.org> wrote:

> I would like this tested on all the Unixes, not just Linux.
> What about the 'open lots of FDs' idea?
>

[bradfitz, 2011-12-19 17:02:38]

Hello golang-dev@googlegroups.com, rsc@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[rsc, 2011-12-19 17:07:22]

http://codereview.appspot.com/5490075/diff/1003/src/pkg/os/exec/exec_test.go
File src/pkg/os/exec/exec_test.go (right):

http://codereview.appspot.com/5490075/diff/1003/src/pkg/os/exec/exec_test.go#...
src/pkg/os/exec/exec_test.go:261: for wantfd := 4; wantfd <= 100; wantfd++ {
try wantfd := os.Stderr.Fd() + 1
It is weird but avoids assuming that fds are ints,
so that this will typecheck on Windows.
We might still need to avoid this test entirely
on Windows but at least this will build.

[rsc, 2011-12-19 17:07:33]

LGTM

[bradfitz, 2011-12-19 17:14:08]

This test is already skipped on Windows.  I'll make the change so it
compiles, though.


On Mon, Dec 19, 2011 at 9:07 AM, <rsc@golang.org> wrote:

>
> http://codereview.appspot.com/**5490075/diff/1003/src/pkg/os/**
>
exec/exec_test.go<http://codereview.appspot.com/5490075/diff/1003/src/pkg/os/exec/exec_test.go>
> File src/pkg/os/exec/exec_test.go (right):
>
> http://codereview.appspot.com/**5490075/diff/1003/src/pkg/os/**
>
exec/exec_test.go#newcode261<http://codereview.appspot.com/5490075/diff/1003/src/pkg/os/exec/exec_test.go#newcode261>
> src/pkg/os/exec/exec_test.go:**261: for wantfd := 4; wantfd <= 100;
> wantfd++ {
> try wantfd := os.Stderr.Fd() + 1
> It is weird but avoids assuming that fds are ints,
> so that this will typecheck on Windows.
> We might still need to avoid this test entirely
> on Windows but at least this will build.
>
>
http://codereview.appspot.com/**5490075/<http://codereview.appspot.com/5490075/>
>

[bradfitz, 2011-12-19 17:23:12]

*** Submitted as http://code.google.com/p/go/source/detail?r=f2d3849606f5 ***

exec: add test to verify net package's epoll fd doesn't go to child

R=rsc
CC=golang-dev
http://codereview.appspot.com/5490075

[bradfitz, 2011-12-19 17:24:34]

Ian,

I submitted this with the test disabled.  When you submit the CLOEXEC
change, remove the skip?

I noticed it fails on both Darwin and Linux so the epoll fix alone isn't
enough.


On Mon, Dec 19, 2011 at 9:23 AM, <bradfitz@golang.org> wrote:

> *** Submitted as
>
http://code.google.com/p/go/**source/detail?r=f2d3849606f5<http://code.google...
>
> exec: add test to verify net package's epoll fd doesn't go to child
>
> R=rsc
> CC=golang-dev
> http://codereview.appspot.com/**5490075<http://codereview.appspot.com/5490075>
>
>
>
http://codereview.appspot.com/**5490075/<http://codereview.appspot.com/5490075/>
>