diff --git a/conn_deadline_test.go b/conn_deadline_test.go new file mode 100644 index 0000000..72f1b99 --- /dev/null +++ b/conn_deadline_test.go @@ -0,0 +1,52 @@ +package zeroconf + +import ( + "net" + "testing" + "time" + + "github.com/enbility/zeroconf/v3/api" + "golang.org/x/net/ipv4" + "golang.org/x/net/ipv6" +) + +// deadlineConn records the write deadline set on a real UDP socket +type deadlineConn struct { + *net.UDPConn + deadline time.Time +} + +func (c *deadlineConn) SetWriteDeadline(t time.Time) error { + c.deadline = t + return c.UDPConn.SetWriteDeadline(t) +} + +// every write must be bounded, a stuck socket would otherwise stall the caller forever +func TestWriteToSetsDeadline(t *testing.T) { + for _, tc := range []struct { + network, addr string + wrap func(net.PacketConn) api.PacketConn + }{ + {"udp4", "127.0.0.1:0", func(c net.PacketConn) api.PacketConn { return newIPv4PacketConn(ipv4.NewPacketConn(c)) }}, + {"udp6", "[::1]:0", func(c net.PacketConn) api.PacketConn { return newIPv6PacketConn(ipv6.NewPacketConn(c)) }}, + } { + t.Run(tc.network, func(t *testing.T) { + udp, err := net.ListenPacket(tc.network, tc.addr) + if err != nil { + t.Skipf("%s not available: %v", tc.network, err) + } + t.Cleanup(func() { _ = udp.Close() }) + + conn := &deadlineConn{UDPConn: udp.(*net.UDPConn)} + start := time.Now() + + if _, err := tc.wrap(conn).WriteTo([]byte("x"), 0, udp.LocalAddr()); err != nil { + t.Fatalf("write: %v", err) + } + + if conn.deadline.Before(start) || conn.deadline.After(time.Now().Add(writeTimeout)) { + t.Fatalf("deadline %v not within write timeout of %v", conn.deadline, start) + } + }) + } +} diff --git a/conn_ipv4.go b/conn_ipv4.go index 6acc847..7457a7d 100644 --- a/conn_ipv4.go +++ b/conn_ipv4.go @@ -5,11 +5,15 @@ import ( "net" "runtime" "syscall" + "time" "github.com/enbility/zeroconf/v3/api" "golang.org/x/net/ipv4" ) +// writeTimeout bounds a single multicast write, a healthy interface completes it in microseconds +const writeTimeout = 100 * time.Millisecond + // ipv4PacketConn wraps ipv4.PacketConn to implement api.PacketConn interface. // This adapter is needed because ipv4.PacketConn uses ControlMessage for // interface selection, but we only need the IfIndex field. @@ -64,6 +68,12 @@ func (c *ipv4PacketConn) WriteTo(b []byte, ifIndex int, dst net.Addr) (n int, er } } + // a multicast write can block forever when the socket stops draining (seen on macOS with + // many interfaces), stalling every later announcement and Shutdown: drop the packet instead + if err := c.conn.SetWriteDeadline(time.Now().Add(writeTimeout)); err != nil { + return 0, err + } + return c.conn.WriteTo(b, cm, dst) } diff --git a/conn_ipv6.go b/conn_ipv6.go index c723148..dbb7c5d 100644 --- a/conn_ipv6.go +++ b/conn_ipv6.go @@ -5,6 +5,7 @@ import ( "net" "runtime" "syscall" + "time" "github.com/enbility/zeroconf/v3/api" "golang.org/x/net/ipv6" @@ -64,6 +65,12 @@ func (c *ipv6PacketConn) WriteTo(b []byte, ifIndex int, dst net.Addr) (n int, er } } + // a multicast write can block forever when the socket stops draining (seen on macOS with + // many interfaces), stalling every later announcement and Shutdown: drop the packet instead + if err := c.conn.SetWriteDeadline(time.Now().Add(writeTimeout)); err != nil { + return 0, err + } + return c.conn.WriteTo(b, cm, dst) } diff --git a/error_classify.go b/error_classify.go index d70fb8d..4fbbec2 100644 --- a/error_classify.go +++ b/error_classify.go @@ -2,6 +2,7 @@ package zeroconf import ( "errors" + "os" "strings" "syscall" ) @@ -39,6 +40,11 @@ func isInterfaceGone(err error) bool { return false } + // a timed out write would stall the next send again, back off the interface like a gone one + if errors.Is(err, os.ErrDeadlineExceeded) { + return true + } + // Check known error codes for _, e := range interfaceGoneErrors { if errors.Is(err, e) { diff --git a/error_classify_test.go b/error_classify_test.go index 1b37258..841bb7a 100644 --- a/error_classify_test.go +++ b/error_classify_test.go @@ -3,6 +3,8 @@ package zeroconf import ( "errors" "fmt" + "net" + "os" "syscall" "testing" ) @@ -129,3 +131,11 @@ func TestIsInterfaceGone_FallbackMessageParsing_CaseInsensitive(t *testing.T) { t.Errorf("expected true for uppercase 'NETWORK IS DOWN', got false") } } + +func TestIsInterfaceGone_WriteDeadlineExceeded_ReturnsTrue(t *testing.T) { + // a multicast write that timed out never drains, the interface must back off + err := &net.OpError{Op: "write", Net: "udp", Err: os.ErrDeadlineExceeded} + if !isInterfaceGone(err) { + t.Errorf("expected true for write deadline exceeded, got false") + } +}