code review 49490043: net: ignore some errors in windows Accept

net: ignore some errors in windows Accept

Fixes issue 6987

[brainman, 2014-01-09 02:21:16]

Hello golang-codereviews@googlegroups.com,

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

[dvyukov, 2014-01-09 09:43:06]

https://codereview.appspot.com/49490043/diff/40001/src/pkg/net/fd_windows.go
File src/pkg/net/fd_windows.go (right):

https://codereview.appspot.com/49490043/diff/40001/src/pkg/net/fd_windows.go#...
src/pkg/net/fd_windows.go:582: case syscall.ERROR_NETNAME_DELETED,
syscall.WSAECONNRESET:
please add a short description as to why we ignore each error
I would prefer if you change the TODO to just a comment. Such lingering TODOs
disturb during code reading, TODO implies that there is something to do but
nobody knows what exactly.

https://codereview.appspot.com/49490043/diff/40001/src/pkg/net/net_windows_te...
File src/pkg/net/net_windows_test.go (right):

https://codereview.appspot.com/49490043/diff/40001/src/pkg/net/net_windows_te...
src/pkg/net/net_windows_test.go:20: func startDialerProcess(t *testing.T, dir
string, addr string) *exec.Cmd {
Doesn't

for i := 0; i < 1000; i++ {
  c := Dial(...)
  c.Close()
}

achieve the goal?

If not, it may be simpler to start:
GOTEST_DIAL_AT_PORT=12345 os.Args[0] -test.run=TestMyTest
and then check for GOTEST_DIAL_AT_PORT at the beginning of the test
rather than compiling a separate program.

[brainman, 2014-01-10 03:36:35]

https://codereview.appspot.com/49490043/diff/40001/src/pkg/net/fd_windows.go
File src/pkg/net/fd_windows.go (right):

https://codereview.appspot.com/49490043/diff/40001/src/pkg/net/fd_windows.go#...
src/pkg/net/fd_windows.go:582: case syscall.ERROR_NETNAME_DELETED,
syscall.WSAECONNRESET:
On 2014/01/09 09:43:07, dvyukov wrote:
> please add a short description as to why we ignore each error
> I would prefer if you change the TODO to just a comment. Such lingering TODOs
> disturb during code reading, TODO implies that there is something to do but
> nobody knows what exactly.

Done.

https://codereview.appspot.com/49490043/diff/40001/src/pkg/net/net_windows_te...
File src/pkg/net/net_windows_test.go (right):

https://codereview.appspot.com/49490043/diff/40001/src/pkg/net/net_windows_te...
src/pkg/net/net_windows_test.go:20: func startDialerProcess(t *testing.T, dir
string, addr string) *exec.Cmd {
On 2014/01/09 09:43:07, dvyukov wrote:
> Doesn't
> 
> for i := 0; i < 1000; i++ {
>   c := Dial(...)
>   c.Close()
> }
> 
> achieve the goal?

I tried this:

diff --git a/src/pkg/net/net_windows_test.go b/src/pkg/net/net_windows_test.go
new file mode 100644
--- /dev/null
+++ b/src/pkg/net/net_windows_test.go
@@ -0,0 +1,34 @@
+// Copyright 2014 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package net
+
+import (
+	"testing"
+)
+
+func TestAcceptIgnoreSomeErrors(t *testing.T) {
+	ln, err := Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("Listen failed: %v", err)
+	}
+	defer ln.Close()
+
+	for i := 1; i < 100; i++ {
+		c, err := Dial("tcp", ln.Addr().String())
+		if err != nil {
+			t.Fatalf("Dial failed: %v", err)
+		}
+		c.Close()
+	}
+
+	for i := 1; i < 100; i++ {
+		c, err := ln.Accept()
+		if err != nil {
+			t.Fatalf("Accept failed: %v", err)
+		}
+		c.Close()
+	}
+
+}

with no luck.

> 
> If not, it may be simpler to start:
> GOTEST_DIAL_AT_PORT=12345 os.Args[0] -test.run=TestMyTest
> and then check for GOTEST_DIAL_AT_PORT at the beginning of the test
> rather than compiling a separate program.

Agreed. Changed the test.

[brainman, 2014-01-10 03:36:54]

Hello golang-codereviews@googlegroups.com, dvyukov@google.com (cc:
golang-codereviews@googlegroups.com),

Please take another look.

[dvyukov, 2014-01-10 09:02:09]

LGTM

[dvyukov, 2014-01-10 09:02:15]

https://codereview.appspot.com/49490043/diff/60001/src/pkg/net/net_windows_te...
File src/pkg/net/net_windows_test.go (right):

https://codereview.appspot.com/49490043/diff/60001/src/pkg/net/net_windows_te...
src/pkg/net/net_windows_test.go:19: 
delete empty line

[brainman, 2014-01-12 01:20:25]

*** Submitted as https://code.google.com/p/go/source/detail?r=7ecbc2b8ec97 ***

net: ignore some errors in windows Accept

Fixes issue 6987

R=golang-codereviews, dvyukov
CC=golang-codereviews
https://codereview.appspot.com/49490043

https://codereview.appspot.com/49490043/diff/60001/src/pkg/net/net_windows_te...
File src/pkg/net/net_windows_test.go (right):

https://codereview.appspot.com/49490043/diff/60001/src/pkg/net/net_windows_te...
src/pkg/net/net_windows_test.go:19: 
On 2014/01/10 09:02:15, dvyukov wrote:
> delete empty line

Done.

[brainman, 2014-01-12 01:21:26]

On 2014/01/10 09:02:15, dvyukov wrote:
> delete empty line

Done.