recover from double close

Author: tonymet

multilistener/multilistener.go

@@ -2,8 +2,8 @@
 Package multilistener -- listen to all loopback interfaces, or a mixed slice of
 ipv4 & ipv6 interfaces
 
-go std library (net.Listen("tcp", ":8080")) can listen to ALL interfaces 
-but cannot listen to all local interfaces by default.  
+go std library (net.Listen("tcp", ":8080")) can listen to ALL interfaces
+but cannot listen to all local interfaces by default.
 
 use multilistener.ListenLocalLoopback to return a single Listener for all ipv4 &
 ipv6 loopback interfaces
@@ -19,7 +19,7 @@ import (
 	"sync"
 )
 
-// MultiListener implements net.Listener interface 
+// MultiListener implements net.Listener interface
 //
 // multiplexes multiple net.Listeners concurrently looping over Accept()
 type MultiListener struct {
@@ -64,7 +64,7 @@ func NewLocalLoopback(port string) (*MultiListener, error) {
 }
 
 // NewMultiListenerRaw returns a MultiListener wrapper of multiple listeners
-// 
+//
 // useful when raw net.Addr or net.Listener is needed
 func NewMultiListenerRaw(listeners []net.Listener) (*MultiListener, error) {
 	dl := &MultiListener{
@@ -79,7 +79,7 @@ func NewMultiListenerRaw(listeners []net.Listener) (*MultiListener, error) {
 	return dl, nil
 }
 
-// NewMultiListener returns multilistener over slice of []string 
+// NewMultiListener returns multilistener over slice of []string
 //
 // see net.Dial and net.Listen for the string format of the address
 // e.g. "[::1]:8080" for ipv6 and "127.0.0.1:8080" for ipv4
@@ -138,8 +138,13 @@ func (dl *MultiListener) Accept() (net.Conn, error) {
 
 // Close closes all internal channels
 //
-// do not defer Close() if passing to http.Server 
-func (dl *MultiListener) Close() error {
+// do not defer Close() if passing to http.Server
+func (dl *MultiListener) Close() (err error) {
+	defer func() {
+		if r := recover(); r != nil {
+			err = fmt.Errorf("MultiListener.Close() panicked on second call: %v", r)
+		}
+	}()
 	close(dl.closeCh)
 	// todo clean up ugly
 	var firstErr error

multilistener/multilistener_test.go

@@ -14,7 +14,7 @@ import (
 // AI claimed IPV6 ::1 C sockets also accepted ipv4.  This test confirms go sockets do not
 // see also cmd/simple-listener and test with telnet
 // I feel like I'm taking crazy pills.
-func TestListenOnlyIPV6(t *testing.T){
+func TestListenOnlyIPV6(t *testing.T) {
 	ready := make(chan struct{})
 	port := "8084"
 	ml, err := net.Listen("tcp", "[::1]:"+port) // Listen only on IPv6 loopback
@@ -35,7 +35,7 @@ func TestListenOnlyIPV6(t *testing.T){
 	// server is ready
 	<-ready
 	// Attempt to connect to the IPv4 loopback address
-	testAddr := "127.0.0.1:"+port
+	testAddr := "127.0.0.1:" + port
 	t.Logf("Attempting to connect to %s (IPv4) to an IPv6-only listener", testAddr)
 	resp, err := http.Get("http://" + testAddr)
 	if err != nil {
@@ -48,21 +48,20 @@ func TestListenOnlyIPV6(t *testing.T){
 	t.Fatalf("Unexpected success connecting to IPv4 address %s on an IPv6-only listener. Status: %d", testAddr, resp.StatusCode)
 }
 
-
 func TestMultiListener(t *testing.T) {
 	ready := make(chan struct{})
-	ml, err := NewLocalLoopback("8081") // Use port 0 to get a random available port
+	ml, err := NewLocalLoopback("0") // Use port 0 to get a random available port
 	if err != nil {
 		t.Fatalf("Failed to create MultiListener: %v", err)
 	}
 	// Start an HTTP server on the MultiListener
 	go func() {
 		close(ready)
 		if err := http.Serve(ml, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			if _, err := io.WriteString(w, "Hello from MultiListener!"); err != nil{
+			if _, err := io.WriteString(w, "Hello from MultiListener!"); err != nil {
 				t.Errorf("error writeString")
 			}
-		})); err != nil{
+		})); err != nil {
 			t.Errorf("http serve error")
 		}
 	}()
@@ -102,17 +101,72 @@ func TestMultiListener(t *testing.T) {
 	}
 }
 
-
 // NewLocalLoopback  when you want to listen to ipv6 & ipv4 loopback with one listener
 func ExampleNewLocalLoopback() {
-	dual, err := NewLocalLoopback("8080")
+	http.HandleFunc("/bar", func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprint(w, "Hello World!\n")
+	})
+
+	dual, err := NewLocalLoopback("8129")
 	if err != nil {
 		panic(err)
 	}
 	fmt.Printf("Serving HTTP %+v\n", dual.AllAddr())
 	fmt.Printf("Preferred Addr: %+v\n", dual.Addr())
 	go http.Serve(dual, nil) //nolint:errcheck
 	// Output:
-	// Serving HTTP [::1]:8080,127.0.0.1:8080
-	// Preferred Addr: [::1]:8080
-}
+	// Serving HTTP [::1]:8129,127.0.0.1:8129
+	// Preferred Addr: [::1]:8129
+}
+
+func TestMultiListenerDoubleClose(t *testing.T) {
+	ml, err := NewLocalLoopback("0")
+	if err != nil {
+		t.Fatalf("Failed to create MultiListener: %v", err)
+	}
+
+	// First close should be successful
+	err = ml.Close()
+	if err != nil {
+		t.Errorf("First close failed unexpectedly: %v", err)
+	}
+
+	// Second close should ideally not panic and return an error or be a no-op
+	// We use a defer func to recover from a potential panic
+	defer func() {
+		if r := recover(); r != nil {
+			t.Errorf("MultiListener.Close() panicked on second call: %v", r)
+		}
+	}()
+
+	err = ml.Close()
+	if err == nil {
+		t.Errorf("Second close succeeded unexpectedly, should return an error or be a no-op")
+	}
+	t.Logf("Second close returned: %v (expected error or no-op)", err)
+}
+
+func TestMultiListenerCloseBeforeServe(t *testing.T) {
+	//t.Skip()
+	ml, err := NewLocalLoopback("0")
+	if err != nil {
+		t.Fatalf("Failed to create MultiListener: %v", err)
+	}
+
+	// Close the listener immediately after creation, before serving
+	err = ml.Close()
+	if err != nil {
+		t.Fatalf("Failed to close MultiListener before serving: %v", err)
+	}
+
+	// Attempt to serve on a closed listener, should return an error
+	serveErr := http.Serve(ml, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Errorf("Handler should not be called on a closed listener")
+	}))
+
+	if serveErr == nil {
+		t.Errorf("http.Serve on a closed listener did not return an error")
+	} else {
+		t.Logf("http.Serve on a closed listener returned expected error: %v", serveErr)
+	}
+}