code review 5269044: http: shut up a false Transport warning on Windows

http: shut up a false Transport warning on Windows

Fixes issue 2057

[bradfitz, 2011-10-14 18:10:10]

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

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

[bradfitz, 2011-10-14 18:12:16]

I'm sure this isn't comprehensive but it fixes it for my Windows system, and
it at least makes forward progress.

Once this is in, the Windows team can expand on it to add symbols for the
couple other errors that can come up (NET_NAME_DELETED, etc?).  I only see
this one on Windows 7, though, but I can't find the symbol.


On Fri, Oct 14, 2011 at 11:10 AM, <bradfitz@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> http: shut up a false Transport warning on Windows
>
> Fixes issue 2057
>
> Please review this at
http://codereview.appspot.com/**5269044/<http://codereview.appspot.com/5269044/>
>
> Affected files:
>  M src/pkg/http/Makefile
>  M src/pkg/http/transport.go
>  A src/pkg/http/transport_**windows.go
>
>
> Index: src/pkg/http/Makefile
> ==============================**==============================**=======
> --- a/src/pkg/http/Makefile
> +++ b/src/pkg/http/Makefile
> @@ -24,4 +24,9 @@
>        transfer.go\
>        transport.go\
>
> +GOFILES_windows=\
> +       transport_windows.go\
> +
> +GOFILES+=$(GOFILES_$(GOOS))
> +
>  include ../../Make.pkg
> Index: src/pkg/http/transport.go
> ==============================**==============================**=======
> --- a/src/pkg/http/transport.go
> +++ b/src/pkg/http/transport.go
> @@ -489,12 +489,24 @@
>        return pc.numExpectedResponses > 0
>  }
>
> +var remoteSideClosedFunc func(os.Error) bool // or nil to use default
> +
> +func remoteSideClosed(err os.Error) bool {
> +       if err == os.EOF || err == os.EINVAL {
> +               return true
> +       }
> +       if remoteSideClosedFunc != nil {
> +               return remoteSideClosedFunc(err)
> +       }
> +       return false
> +}
> +
>  func (pc *persistConn) readLoop() {
>        alive := true
>        for alive {
>                pb, err := pc.br.Peek(1)
>                if err != nil {
> -                       if (err == os.EOF || err == os.EINVAL) &&
> !pc.expectingResponse() {
> +                       if remoteSideClosed(err) && !pc.expectingResponse()
> {
>                                // Remote side closed on us.  (We probably
> hit their
>                                // max idle timeout)
>                                pc.close()
> Index: src/pkg/http/transport_**windows.go
> ==============================**==============================**=======
> new file mode 100644
> --- /dev/null
> +++ b/src/pkg/http/transport_**windows.go
> @@ -0,0 +1,21 @@
> +// Copyright 2011 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 http
> +
> +import (
> +       "os"
> +       "net"
> +)
> +
> +func init() {
> +       remoteSideClosedFunc = func(err os.Error) (out bool) {
> +               op, ok := err.(*net.OpError)
> +               if ok && op.Op == "WSARecv" && op.Net == "tcp" && op.Error
> == os.Errno(10058) {
> +                       // TODO(bradfitz): find the symbol for 10058
> +                       return true
> +               }
> +               return false
> +       }
> +}
>
>
>

[rsc, 2011-10-14 18:24:34]

LGTM i guess

[bradfitz, 2011-10-14 18:31:03]

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

http: shut up a false Transport warning on Windows

Fixes issue 2057

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

[frichter, 2011-10-15 05:27:39]

FYI - from  http://msdn.microsoft.com/en-us/library/ms819775.aspx

//
// MessageId: WSAESHUTDOWN
//
// MessageText:
//
//  A request to send or receive data was disallowed because the socket had
already been shut down in that direction with a previous shutdown call.
//#define WSAESHUTDOWN                     10058L

[brainman, 2011-10-15 05:33:27]

LGTM

http://codereview.appspot.com/5269044/diff/3004/src/pkg/http/transport_window...
File src/pkg/http/transport_windows.go (right):

http://codereview.appspot.com/5269044/diff/3004/src/pkg/http/transport_window...
src/pkg/http/transport_windows.go:16: // TODO(bradfitz): find the symbol for
10058
It is WSAESHUTDOWN. (I just googled for "10058 sockets"). I will fix it later.

[hector, 2011-10-15 11:05:53]

Might want to add ECONNRESET to the common case as well.