Bound multicast writes so a stuck socket cannot hang Shutdown

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)
+			}
+		})
+	}
+}

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)
 }
 

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)
 }
 

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) {

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")
+	}
+}