From fcbc168ae67fb418d36dfbe0062f888eee8f246d Mon Sep 17 00:00:00 2001 From: Cenk Alti Date: Tue, 25 Dec 2018 11:23:08 +0300 Subject: [PATCH 1/5] add test case for demonstrating panic --- frontend/udp/frontend_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 frontend/udp/frontend_test.go diff --git a/frontend/udp/frontend_test.go b/frontend/udp/frontend_test.go new file mode 100644 index 0000000..b4cff14 --- /dev/null +++ b/frontend/udp/frontend_test.go @@ -0,0 +1,28 @@ +package udp_test + +import ( + "testing" + + "github.com/chihaya/chihaya/frontend/udp" + "github.com/chihaya/chihaya/middleware" + "github.com/chihaya/chihaya/storage" + _ "github.com/chihaya/chihaya/storage/memory" +) + +func TestStartStopRaceIssue437(t *testing.T) { + ps, err := storage.NewPeerStore("memory", nil) + if err != nil { + t.Fatal(err) + } + var responseConfig middleware.ResponseConfig + lgc := middleware.NewLogic(responseConfig, ps, nil, nil) + fe, err := udp.NewFrontend(lgc, udp.Config{Addr: "127.0.0.1:0"}) + if err != nil { + t.Fatal(err) + } + errC := fe.Stop() + errs := <-errC + if len(errs) != 0 { + t.Fatal(errs[0]) + } +} From 1b7ce4c378bfa13f147c386e28f8f1774bc29b4d Mon Sep 17 00:00:00 2001 From: Cenk Alti Date: Tue, 25 Dec 2018 11:23:47 +0300 Subject: [PATCH 2/5] protect socket variable with mutex; fix #437 --- frontend/udp/frontend.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/frontend/udp/frontend.go b/frontend/udp/frontend.go index bc5b421..1c0f32d 100644 --- a/frontend/udp/frontend.go +++ b/frontend/udp/frontend.go @@ -49,6 +49,7 @@ func (cfg Config) LogFields() log.Fields { // Frontend holds the state of a UDP BitTorrent Frontend. type Frontend struct { socket *net.UDPConn + m sync.Mutex closing chan struct{} wg sync.WaitGroup @@ -103,10 +104,20 @@ func (t *Frontend) Stop() stop.Result { c := make(stop.Channel) go func() { + t.m.Lock() + defer t.m.Unlock() + close(t.closing) - t.socket.SetReadDeadline(time.Now()) + if t.socket != nil { + t.socket.SetReadDeadline(time.Now()) + } t.wg.Wait() - c.Done(t.socket.Close()) + + var err error + if t.socket != nil { + err = t.socket.Close() + } + c.Done(err) }() return c.Result() @@ -120,7 +131,9 @@ func (t *Frontend) listenAndServe() error { return err } + t.m.Lock() t.socket, err = net.ListenUDP("udp", udpAddr) + t.m.Unlock() if err != nil { return err } From 2a26215f2afb432cbb106e750149e1223799921b Mon Sep 17 00:00:00 2001 From: Cenk Alti Date: Wed, 26 Dec 2018 18:10:48 +0300 Subject: [PATCH 3/5] Revert "protect socket variable with mutex; fix #437" This reverts commit 1b7ce4c378bfa13f147c386e28f8f1774bc29b4d. --- frontend/udp/frontend.go | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/frontend/udp/frontend.go b/frontend/udp/frontend.go index 1c0f32d..bc5b421 100644 --- a/frontend/udp/frontend.go +++ b/frontend/udp/frontend.go @@ -49,7 +49,6 @@ func (cfg Config) LogFields() log.Fields { // Frontend holds the state of a UDP BitTorrent Frontend. type Frontend struct { socket *net.UDPConn - m sync.Mutex closing chan struct{} wg sync.WaitGroup @@ -104,20 +103,10 @@ func (t *Frontend) Stop() stop.Result { c := make(stop.Channel) go func() { - t.m.Lock() - defer t.m.Unlock() - close(t.closing) - if t.socket != nil { - t.socket.SetReadDeadline(time.Now()) - } + t.socket.SetReadDeadline(time.Now()) t.wg.Wait() - - var err error - if t.socket != nil { - err = t.socket.Close() - } - c.Done(err) + c.Done(t.socket.Close()) }() return c.Result() @@ -131,9 +120,7 @@ func (t *Frontend) listenAndServe() error { return err } - t.m.Lock() t.socket, err = net.ListenUDP("udp", udpAddr) - t.m.Unlock() if err != nil { return err } From b345eb38998bf332381f803fdbf95edf5a6ccdf4 Mon Sep 17 00:00:00 2001 From: Cenk Alti Date: Wed, 26 Dec 2018 18:15:05 +0300 Subject: [PATCH 4/5] split listenAndServe into 2 functions --- frontend/udp/frontend.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/frontend/udp/frontend.go b/frontend/udp/frontend.go index bc5b421..5831213 100644 --- a/frontend/udp/frontend.go +++ b/frontend/udp/frontend.go @@ -84,8 +84,13 @@ func NewFrontend(logic frontend.TrackerLogic, cfg Config) (*Frontend, error) { }, } + err := f.listen() + if err != nil { + return nil, err + } + go func() { - if err := f.listenAndServe(); err != nil { + if err := f.serve(); err != nil { log.Fatal("failed while serving udp", log.Err(err)) } }() @@ -112,19 +117,18 @@ func (t *Frontend) Stop() stop.Result { return c.Result() } -// listenAndServe blocks while listening and serving UDP BitTorrent requests -// until Stop() is called or an error is returned. -func (t *Frontend) listenAndServe() error { +func (t *Frontend) listen() error { udpAddr, err := net.ResolveUDPAddr("udp", t.Addr) if err != nil { return err } - t.socket, err = net.ListenUDP("udp", udpAddr) - if err != nil { - return err - } + return err +} +// listenAndServe blocks while listening and serving UDP BitTorrent requests +// until Stop() is called or an error is returned. +func (t *Frontend) serve() error { pool := bytepool.New(2048) t.wg.Add(1) From 0de1d25448a2bead7290b6ac441f6f113baf67a1 Mon Sep 17 00:00:00 2001 From: Cenk Alti Date: Thu, 27 Dec 2018 15:17:43 +0300 Subject: [PATCH 5/5] fix listenAndServe comment --- frontend/udp/frontend.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frontend/udp/frontend.go b/frontend/udp/frontend.go index 5831213..a75430e 100644 --- a/frontend/udp/frontend.go +++ b/frontend/udp/frontend.go @@ -117,6 +117,7 @@ func (t *Frontend) Stop() stop.Result { return c.Result() } +// listen resolves the address and binds the server socket. func (t *Frontend) listen() error { udpAddr, err := net.ResolveUDPAddr("udp", t.Addr) if err != nil { @@ -126,7 +127,7 @@ func (t *Frontend) listen() error { return err } -// listenAndServe blocks while listening and serving UDP BitTorrent requests +// serve blocks while listening and serving UDP BitTorrent requests // until Stop() is called or an error is returned. func (t *Frontend) serve() error { pool := bytepool.New(2048) @@ -138,7 +139,7 @@ func (t *Frontend) serve() error { // Check to see if we need to shutdown. select { case <-t.closing: - log.Debug("udp listenAndServe() received shutdown signal") + log.Debug("udp serve() received shutdown signal") return nil default: }