浏览代码

Fix SSDP shutdown race and make Close idempotent

Refactors SSDP host lifecycle to stop the alive loop cleanly before/while closing the advertiser, preventing post-close Alive calls that could panic during shutdown (e.g., Ctrl+C). `Start` now uses a ticker with explicit quit/done signaling, and `Close` is guarded with `sync.Once` so repeated calls are safe and non-blocking. Adds a regression test to verify the alive loop exits on close and that calling `Close` twice does not hang or panic.
Toby Chui 5 天之前
父节点
当前提交
fe6379bc76
共有 2 个文件被更改,包括 59 次插入 和 15 次删除
  1. 30 15
      src/mod/network/ssdp/ssdp.go
  2. 29 0
      src/mod/network/ssdp/ssdp_test.go

+ 30 - 15
src/mod/network/ssdp/ssdp.go

@@ -8,6 +8,7 @@ import (
 	"runtime"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	ssdp "github.com/koron/go-ssdp"
@@ -31,7 +32,10 @@ type SSDPHost struct {
 	advStarted       bool
 	SSDPTemplateFile string
 	Option           *SSDPOption
+	aliveInterval    time.Duration
 	quit             chan bool
+	done             chan struct{}
+	closeOnce        sync.Once
 }
 
 func NewSSDPHost(outboundIP string, port int, templateFile string, option SSDPOption) (*SSDPHost, error) {
@@ -65,6 +69,7 @@ func NewSSDPHost(outboundIP string, port int, templateFile string, option SSDPOp
 	return &SSDPHost{
 		ADV:              ad,
 		advStarted:       false,
+		aliveInterval:    5 * time.Second,
 		SSDPTemplateFile: templateFile,
 		Option:           &option,
 	}, nil
@@ -74,36 +79,46 @@ func (a *SSDPHost) Start() {
 	//Advertise ssdp
 	http.HandleFunc("/ssdp.xml", a.handleSSDP)
 	logger.PrintAndLog("Ssdp", "Starting SSDP Discovery Service: "+a.Option.URLBase, nil)
-	var aliveTick <-chan time.Time
-	aliveTick = time.Tick(time.Duration(5) * time.Second)
 
-	quit := make(chan bool)
-	a.quit = quit
+	a.quit = make(chan bool)
+	a.done = make(chan struct{})
 	a.advStarted = true
-	go func(ad *ssdp.Advertiser) {
+	go func(ad *ssdp.Advertiser, quit chan bool, done chan struct{}) {
+		defer close(done)
+		aliveTicker := time.NewTicker(a.aliveInterval)
+		defer aliveTicker.Stop()
 		for {
 			select {
-			case <-aliveTick:
+			case <-aliveTicker.C:
 				if ad != nil {
 					ad.Alive()
 				}
-
 			case <-quit:
-				ad.Bye()
-				ad.Close()
-				break
+				//Return (not break) so no Alive() can run after the advertiser is closed
+				if ad != nil {
+					ad.Bye()
+					ad.Close()
+				}
+				return
 			}
 		}
-	}(a.ADV)
+	}(a.ADV, a.quit, a.done)
 }
 
+// Close sends the SSDP byebye message and stops the advertiser. It waits for
+// the alive loop to exit and is safe to call more than once.
 func (a *SSDPHost) Close() {
-	if a != nil {
+	if a == nil {
+		return
+	}
+	a.closeOnce.Do(func() {
 		if a.advStarted {
-			a.quit <- true
+			close(a.quit)
+			<-a.done
+		} else if a.ADV != nil {
+			a.ADV.Close()
 		}
-	}
-
+	})
 }
 
 // Serve the xml file with the given properties

+ 29 - 0
src/mod/network/ssdp/ssdp_test.go

@@ -3,6 +3,7 @@ package ssdp
 import (
 	"runtime"
 	"testing"
+	"time"
 )
 
 // TestSSDPOption_Fields verifies that an SSDPOption struct can be created with
@@ -93,3 +94,31 @@ func TestNewSSDPHost_SkipIfNoNetwork(t *testing.T) {
 		t.Error("expected advStarted to be false before Start() is called")
 	}
 }
+
+// TestSSDPHost_CloseStopsAliveLoop is a regression test for the "send on closed
+// channel" panic on Ctrl+C: after Close() the alive loop must exit instead of
+// calling Alive() on the closed advertiser at the next tick.
+func TestSSDPHost_CloseStopsAliveLoop(t *testing.T) {
+	host, err := NewSSDPHost("127.0.0.1", 18081, "/nonexistent/template.xml", SSDPOption{UUID: "test-uuid-ssdp-0002"})
+	if err != nil {
+		t.Skipf("skipping SSDP shutdown test (network unavailable): %v", err)
+	}
+	host.aliveInterval = 10 * time.Millisecond
+	host.Start()
+	time.Sleep(30 * time.Millisecond)
+
+	closed := make(chan struct{})
+	go func() {
+		host.Close()
+		host.Close() //A second Close must not block or panic
+		close(closed)
+	}()
+	select {
+	case <-closed:
+	case <-time.After(5 * time.Second):
+		t.Fatalf("Close did not return")
+	}
+
+	//Several ticks after Close; the old loop panicked here
+	time.Sleep(50 * time.Millisecond)
+}