net: UDPConn.ReadFrom failure on Windows

[gopherbot]

by junshi04:

The bug description is below. The program to reproduce the bug is attached.

On Windows, net.UDPConn.ReadFrom may return an error of "WSARecvFrom
...: No service is operating at the destination network endpoint on
the remote system", when the same UDPConn is used to write a packet
that has no recipient.

To reproduce the error, set "-me" to the local IP:port, "-peer" to a
random IP:port, and run the attached file. The ReadFrom will fail
immediately with the "No service is operating ..." error.

If "-peer" is not set, the net.UDPConn is not used to write packets,
and the ReadFrom won't fail.

If "-peer" is set to an address with a recipient, the ReadFrom won't
fail either.

Looks like the error is actually for the write, writing to the peer
IP:port generates an ICMP error, but the error is returned to the
ReadFrom.

Attachments:

1. readfrombug.go (916 bytes)

[alexbrainman]

Comment 1:

Your program kind of works for me, but there are many unclear points. Perhaps it is best
that you change your program so that is simpler and does not require any parameters or
typing and outputs some data. Then you could tell us what your program outputs and what
you think its output should be instead. Just like the original issue template was:
What steps will reproduce the problem?
1.
2.
3.
What is the expected output? What do you see instead?
Please use labels and text to provide additional information.
Thank you.
Alex

Status changed to WaitingForReply.

[gopherbot]

Comment 2 by junshi04:

I simplified the program. It fails on 64-bit windows (windows 7,
windows server 2008). The ReadFrom returns an error "No service is
operating at the destination network endpoint on the remote system.".
I believe the error is actually for the WriteTo call, as there is no
recipient on the remote address. I think the error is triggered by an
ICMP message from the peer, but I did not capture the packet to
confirm it.
I have seen the same error on 32-bit windows machines occasionally,
but could not find a reliable way to trigger it.

Attachments:

1. readfrombug2.go (353 bytes)

[alexbrainman]

Comment 3:

I don't know much about UDP, but it looks to me that is how Windows handles this
situation.
http://www.gamedev.net/topic/540870-udp-iocp-and-error_port_unreachablewsaeconnreset/
https://groups.google.com/forum/#!topic/microsoft.public.win32.programmer.networks/tTOuZ0a4hDs
http://www.lenholgate.com/blog/2007/12/bug-in-overlapped-udp-port-unreachable-errors.html
What do you propose we should do?
Alex

[gopherbot]

Comment 4 by junshi04:

Thanks for these pointers! It is a Windows quirk.
http://support.microsoft.com/kb/263823 has the details. Though it is
still not clear to me why this shows up reliably only on 64-bit
Windows.
Right now, I have to ignore the error of ReadFrom and retry on
Windows. I cannot think of any other reasonable ways to handle this
kind of error.
But the problem of this ignore-and-retry approach is I have to
distinguish this error from other persistent errors such as reading
from closed descriptor by checking the actual error message. That's
ugly!
ReadFrom on Linux does not have this behavior. Does it make sense to
you to export SIO_UDP_CONNRESET in syscall, so that I can call
WSAIoctl to disable this behavior?
Another option is to make this error temporary, i.e. err.Temporary()
== true, so that I can ignore this error in a cheap and safe way.
Thanks,

[alexbrainman]

Comment 5:

Like I said earlier, I am not an expert in net, so I hope others will help.
Alternatively, feel free to discuss your proposal on golang-nuts newsgroup. If that
won't help, just send a proposed change. I am happy to help with your change once
everyone agree on a direction.
Alex

[rsc]

Comment 6:

Labels changed: added priority-later, go1.2, expertneeded, suggested, removed priority-triage.

Status changed to Accepted.

[rsc]

Comment 7:

It looks like the fix is to retry ReadFrom when this error comes back.
Does anyone have time to write a test to go with https://golang.org/cl/13703043?

[rsc]

Comment 8:

I'd like to see that CL turn into a tested fix in some form (and perhaps the loop needs
to be lower down to handle other kinds of reads?), but we won't hold up Go 1.2 for it.

Labels changed: added go1.2maybe, removed go1.2.

[alexbrainman]

Comment 9:

junshi04,
Sorry, but I am still not clear about what we are fixing here. I wrote this test:
diff --git a/src/pkg/net/udp_test.go b/src/pkg/net/udp_test.go
--- a/src/pkg/net/udp_test.go
+++ b/src/pkg/net/udp_test.go
@@ -9,6 +9,7 @@
    "reflect"
    "runtime"
    "testing"
+   "time"
 )
 
 type resolveUDPAddrTest struct {
@@ -280,3 +281,35 @@
        }
    }
 }
+
+func TestUDPReadFrom(t *testing.T) {
+   const raddr = "54.244.132.193:55555"
+   c, err := Dial("udp", raddr)
+   if err != nil {
+       t.Fatalf("Dial failed: %v", err)
+   }
+   defer c.Close()
+
+   ra, err := ResolveUDPAddr("udp", raddr)
+   if err != nil {
+       t.Fatalf("ResolveUDPAddr failed: %v", err)
+   }
+
+   _, err = c.(*UDPConn).WriteToUDP([]byte("a"), ra)
+   if err == nil {
+       t.Fatal("WriteToUDP should fail")
+   }
+   if err != nil && err.(*OpError).Err != ErrWriteToConnected {
+       t.Fatalf("WriteToUDP should fail as ErrWriteToConnected: %v", err)
+   }
+
+   err = c.SetDeadline(time.Now().Add(100 * time.Millisecond))
+   if err != nil {
+       t.Fatalf("SetDeadline failed: %v", err)
+   }
+   b := make([]byte, 1)
+   _, _, err = c.(*UDPConn).ReadFromUDP(b)
+   if err != nil && !isTimeout(err) {
+       t.Fatalf("ReadFromUDP failed: %v", err)
+   }
+}
and it PASSes on both linux and windows here. Please, change the test, so it PASSes on
one and FAILs on the other. This way we can narrow it down where the differences are and
decide what to do about them.
Thank you.
Alex

Status changed to WaitingForReply.

[gopherbot]

Comment 10 by junshi04:

You should be able to see the error, if you run the program attached
in #2, readfrombug2.go, on 64-bit windows. The program may not fail on
a 32-bit windows.
I used net.ListenUDP, not net.Dial. So the WriteToUDP won't fail in my
program. I don't think you should use WriteToUDP. The write would fail
locally with ErrWriteToConnected. But we want the UDP packet to be
sent to the remote host to trigger an ICMP reset packet. It is the
ICMP reset packet that causes the following ReadFromUDP to fail.
Thanks,

[gopherbot]

Comment 11 by junshi04:

As to the fix, I think a better fix is to disable the
SIO_UDP_CONNRESET behavior, to avoid the bogus read errors.
Thanks,

[alexbrainman]

Comment 12:

I run my new TestUDPReadFrom on both windows/386 and windows/amd64. It PASSes on both.
If you want me to look further, please change my test as requested in
https://golang.org/issue/5834?c=9. I don't know enough about Go udp
code, but why cannot you do what my test does?
Alex

[gopherbot]

Comment 13 by junshi04:

I don't know how to run your test. It is part of the standard "net" package.
As I pointed out in my early message, your test should not use both
net.Dial and WriteToUDP. The WriteToUDP would fail locally, because
you cannot use WriteToUDP on a connected UDP socket. To reproduce the
fail I reported, the UDP packet should be sent to the remote host to
trigger an ICMP reset packet.
You need to make these changes to your test,
- replace Dial with ListenUDP
- check that WriteToUDP does not fail
- the raddr "54.244.132.193" is my EC2 instance. It is up at this
point, but you'd better replace it with a well-known host that
generates ICMP reset packets (www.google.com does not send ICMP reset
packets afaik).
Thanks,

[alexbrainman]

Comment 14:

Like I said earlier, I don't know enough about Go udp. So I need a test that I can use
to develop solution. Please, try again to modify my test, as I have requested.
Here is what I do to modify udp_test.go file to add new test and run it:
C:\>cd %GOROOT%\src\pkg\net
C:\go\root\src\pkg\net>hg st
C:\go\root\src\pkg\net>hg patch -f --no-commit u:\a.diff
applying u:\a.diff
C:\go\root\src\pkg\net>hg st
M src\pkg\net\udp_test.go
C:\go\root\src\pkg\net>go test -v -run=TestUDPReadFrom
=== RUN TestUDPReadFrom
--- PASS: TestUDPReadFrom (0.11 seconds)
PASS
ok      net     0.203s
C:\go\root\src\pkg\net>
a.diff file attached. Please modify the test to demonstrate the issue (it must PASS on
linux and FAIL on windows or vice versa), and post your changes here (output of "hg
diff" command should do).
Thank you.
Alex

Attachments:

1. a.diff (1092 bytes)

[rsc]

Comment 15:

Labels changed: added go1.3, removed go1.2maybe.