From 9fb7b185fcde9b49fa0524b6001127612874f1a4 Mon Sep 17 00:00:00 2001 From: Koala Yeung Date: Thu, 18 Feb 2021 13:58:13 +0800 Subject: [PATCH 1/2] client: properly close down internal idpool on Close * Client.Close() now also close the internal idPool implementation, which effectively close the channel in it. --- client.go | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/client.go b/client.go index 6e9dc66..89bda2c 100644 --- a/client.go +++ b/client.go @@ -18,6 +18,7 @@ import ( "io" "net" "net/http" + "runtime" "strconv" "strings" "sync" @@ -68,19 +69,26 @@ type idPool struct { } // AllocID implements Client.AllocID -func (p *idPool) Alloc() uint16 { +func (p *idPool) Alloc() (id uint16) { return <-p.IDs } // ReleaseID implements Client.ReleaseID func (p *idPool) Release(id uint16) { go func() { + defer recoverSendOnClosedChannel() + // release the ID back to channel for reuse // use goroutine to prev0, ent blocking ReleaseID p.IDs <- id }() } +func (p *idPool) Close() { + + close(p.IDs) +} + func newIDs(limit uint32) (p idPool) { // sanatize limit @@ -342,14 +350,19 @@ func (c *client) Do(req *Request) (resp *ResponsePipe, err error) { } // Close implements Client.Close +// // If the inner connection has been closed before, -// this method would do nothing and return nil +// this method would do nothing and return nil. +// +// This will also close the IDPool. func (c *client) Close() (err error) { + // Close inner connection. if c.conn == nil { return } err = c.conn.Close() c.conn = nil + c.ids.Close() // close the return } @@ -585,3 +598,17 @@ func (c ClientFunc) Do(req *Request) (resp *ResponsePipe, err error) { func (c ClientFunc) Close() error { return nil } + +func recoverSendOnClosedChannel() { + if r := recover(); r != nil { + switch v := r.(type) { + case runtime.Error: + if v.Error() == "send on closed channel" { + // ignore + return + } + default: + panic(r) + } + } +} From 1893199e047cb6cf8714df313a8ebe2183f8c724 Mon Sep 17 00:00:00 2001 From: Koala Yeung Date: Thu, 18 Feb 2021 14:08:12 +0800 Subject: [PATCH 2/2] client: Fix data race in idPool usage * There don't seem to be a need to pass idPool pointer around. --- client.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/client.go b/client.go index 89bda2c..0bf7572 100644 --- a/client.go +++ b/client.go @@ -69,12 +69,12 @@ type idPool struct { } // AllocID implements Client.AllocID -func (p *idPool) Alloc() (id uint16) { +func (p idPool) Alloc() (id uint16) { return <-p.IDs } // ReleaseID implements Client.ReleaseID -func (p *idPool) Release(id uint16) { +func (p idPool) Release(id uint16) { go func() { defer recoverSendOnClosedChannel() @@ -84,8 +84,7 @@ func (p *idPool) Release(id uint16) { }() } -func (p *idPool) Close() { - +func (p idPool) Close() { close(p.IDs) }